Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: #631 revert change in label.rs to fix error labels #645

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ojkelly
Copy link

@ojkelly ojkelly commented Jun 22, 2024

When a source does not contain a newline, labels are lost/ignored and replaced with the fallback expected something else.

This reverts the change made in this commit which appears to fix this 6837537#diff-3d2dfe7e70ee2398dd0941e8f6ea855652695c3f82b7816f649668fb2d56b93dR60

When a source does not contain a newline,  labels are lost/ignored and replaced with the fallback `expected something else`.

This reverts the change made in this commit which appears to fix this zesterer@6837537#diff-3d2dfe7e70ee2398dd0941e8f6ea855652695c3f82b7816f649668fb2d56b93dR60
@zesterer
Copy link
Owner

Hmm. I am a bit suspicious of this, but I need to take another look at it at some point. Do tests continue to pass with --features label?

@ojkelly
Copy link
Author

ojkelly commented Jun 28, 2024

Hmm. I am a bit suspicious of this, but I need to take another look at it at some point.

Yeah me too, I'm not familiar with this part of the codebase. I had a play around with the diffs to see what would fix it for my repo and this has.

Looks like the tests are failing.

@xldenis
Copy link

xldenis commented Jul 18, 2024

I seem to be encountering this issue in alpha.7 when using select! in combination with a logos lexer. I'll need to investigate further to minimize.

@xldenis
Copy link

xldenis commented Jul 24, 2024

I replicated this issue locally with a parser and confirmed this patch fixes it. Additionally, the CI failures seem to be spurious failures caused by time. I think if @zesterer restarts CI it should pass.

@zesterer
Copy link
Owner

I replicated this issue locally with a parser and confirmed this patch fixes it. Additionally, the CI failures seem to be spurious failures caused by time. I think if @zesterer restarts CI it should pass.

As a point of interest, do you have an example of what you think the correct behaviour is that you can turn into a test? That would be very useful for figuring out the best way to implement a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants