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

Add "not matching regex" conditional #2490

Merged
merged 9 commits into from
Dec 11, 2024

Conversation

laniakea64
Copy link
Contributor

Closes #2416

@laniakea64 laniakea64 force-pushed the conditional-not-regex branch from 22641dd to bc176f2 Compare December 3, 2024 15:56
@casey
Copy link
Owner

casey commented Dec 10, 2024

The Unspecified token kind isn't really intended to be surfaced to users, it's for constructing an compile error, which normally requires a token, when there is none.

See the test I added in the last commit, which fails with something like:

error: Expected '&&', '!=', '!~', '||', '==', '=~', '+', or '/', but found unspecified
 ——▶ justfile:1:12
  │
1 │ x := if '' !! '' { '' } else { '' }
  │            ^

I think the signature of lex_choices should be changed to:

  fn lex_choices(
    &mut self,
    first: char,
    choices: &[(char, TokenKind)],
    otherwise: Option<TokenKind>,
  ) -> CompileResult<'src> {

And if otherwise is None, it generates an UnexpectedCharacter error.

@laniakea64
Copy link
Contributor Author

Done and fixed the failing test. Thanks for the hint how to make that error message more sensible 👍

Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM!

@casey casey merged commit 53f8619 into casey:master Dec 11, 2024
5 checks passed
@laniakea64
Copy link
Contributor Author

@casey Actually there is an issue with this approach to the error message:

With the following invalid justfile content:

a := if x !

If this is saved in a justfile without trailing newline, so that the ! is the very last character in the file, just will produce this error -

$ just -l
error: Internal error, this may indicate a bug in just: Lexer advanced past end of text
consider filing an issue: https://github.com/casey/just/issues/new
 ——▶ justfile:1:12
  │
1 │ a := if x !
  │            ^

But this is not a bug in just, advancing one character past the end of text is the expected behavior in this case.

Adding the trailing newline to the justfile results in the expected error message.

An alternative approach I considered when making this PR was adding a Bang token, to enable lexing != and !~ by the exact same approach as the pre-existing lexing of == and =~, so that the error message would look like that one:

$ just -l
error: Expected '&&', '!=', '!~', '||', '==', '=~', '(', '+', or '/', but found '='
 ——▶ justfile:1:11
  │
1 │ a := if x =
  │           ^

I didn't go that route because ! isn't valid syntax anywhere in just, and I had doubts about adding a token that is never valid syntax.

Not sure what would be best fix this (and not sure if I'll have time to make another PR myself ☹️ )

@casey
Copy link
Owner

casey commented Dec 11, 2024

Nice catch! This is an easy fix, there's an UnexpectedEndOfToken error, for just this eventuality. Fixed in #2520.

@laniakea64
Copy link
Contributor Author

Nice catch! This is an easy fix, there's an UnexpectedEndOfToken error, for just this eventuality. Fixed in #2520.

Much better, thanks!

Seems to be some extra backticks in the error message? -

error: Expected character ``=` or `~`` but found end-of-file
 ——▶ justfile:1:12
  │
1 │ a := if x !
  │            ^

@casey
Copy link
Owner

casey commented Dec 11, 2024

Good catch, fixed in #2522!

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.

Conditional operator for NOT match regex?
2 participants