jupyter-xeus / xeus-octave

Jupyter kernel for GNU Octave
https://xeus-octave.readthedocs.io/
GNU General Public License v3.0
57 stars 10 forks source link

New parsing method inspired by octave script parsing code #99

Closed rapgenic closed 1 year ago

rapgenic commented 1 year ago

The parser and code runner inside the execute_request implementation has been simplified.

Inspiration has been taken from the parse_fcn_file in oct_parse.cc (or oct_parse.yy) in Octave codebase.

The errors @AntoinePrv reported are now fixed, and the error is actually reported to the correct line of the cell.

Close: #68

rapgenic commented 1 year ago

The only drawback is that syntax error do not show the wrong code anymore

Execution exception: parse error near line 2 of file cell[1]

  syntax error

[[[ code missing ]]]

When it should to be

Execution exception: parse error:

  syntax error

>>> [][1]
      ^

From exploration of octave code this seems inevitable: either we parse code line by line (as REPL does) but we lose line information or we parse as if the cell was a file, but the bison_error procedure in oct_parse.cc (or oct_parse.yy) tries to read such file in order to display the wrong code, but no file called cell[1] actually exists.

In my opinion this is acceptable, considering the other benefits (fix of previous bug and line information)

rapgenic commented 1 year ago

We could just copy and tweak the function from octave core that generates this error, and replace it manually for parse errors... Not nice, but doable I think... I'll have a look at it.

Anyway octave developers themselves have marked this behaviour as FIXME, as they want to avoid reading a file from the filesystem, so this might actually be fixed in the future (or I might try with a PR, but I'm not sure :smile: )

rapgenic commented 1 year ago

@AntoinePrv this should do it! Not nice but better than nothing