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

Bug in the new syntax error highlighting #2639

Closed
fingolfin opened this issue Jul 12, 2018 · 1 comment
Closed

Bug in the new syntax error highlighting #2639

fingolfin opened this issue Jul 12, 2018 · 1 comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them priority: low topic: kernel

Comments

@fingolfin
Copy link
Member

The code introduced in #2574 does not work quite right if the code parsing gets interrupted by an error. Here is an example:

gap> a:=[1..3];; List(a. x -> x);
Error, Record Element: <rec> must be a record (not a list (range,ssort))
not in any function at *stdin*:1
you can replace <rec> via 'return <rec>;'
brk> quit;
Syntax error: ) expected
a:=[1..3];; List(a. x -> x);
    ^^^^^^^^^^^^^^^^^^^^
gap> a:=[1..3];; List(a. x -> x);
Error, Record Element: <rec> must be a record (not a list (range,ssort))
not in any function at *stdin*:1
you can replace <rec> via 'return <rec>;'
brk> [1,2,3];; quit;
Syntax error: ) expected
a:=[1..3];; List(a. x -> x);
              ^^^^^^^^^^
gap> a:=[1..3];; List(a. x -> x);
Error, Record Element: <rec> must be a record (not a list (range,ssort))
not in any function at *stdin*:1
you can replace <rec> via 'return <rec>;'
brk> [1,2,3,4,5,6,7,8,9];; quit;
Syntax error: ) expected
a:=[1..3];; List(a. x -> x);

As you can see, the length of the underlined segment depends on how much text we entered when leaving the break loop. That's because the break loop of course uses the scanner, too, and hence modifies SymbolStartLine and SymbolStartPos which we don't save and restore in e.g. ReadEvalCommand (or wherever else this might make sense).

I am working on a PR to completely factor out the reader&scanner state, which should solve this issue and more (we actually also don't quite save&restore some other state which we probably should, although I do not yet have test cases to trigger bugs this way). But if anybody wants to make a fix for this in the meantime, please go ahead.

Note that I also think that it is not nice that GAP throws an error here, but I see no good way to avoid that w/o changing that we evaluate immediately in interpreter mode; still, the first error is rather confusing, and the syntax error after leaving the break loop even more so.

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them topic: kernel labels Jul 12, 2018
@fingolfin fingolfin changed the title Bug in the next syntax error highlighting Bug in the new syntax error highlighting Jul 13, 2018
@fingolfin
Copy link
Member Author

This is fixed in current master, yay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them priority: low topic: kernel
Projects
None yet
Development

No branches or pull requests

1 participant