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

Improve generic command #149

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jdujava
Copy link
Contributor

@jdujava jdujava commented Jul 21, 2024

Fixes #146
Fixes #108

It seems to work pretty well, see included tests.

This however goes against #51, thus breaking #48. This can be annoying, even though

\(]0,+\infty[\)

can be fixed manually by

\(\left]0,+\infty\right[\)

\(]0,+\infty{[}\)

\(]0,+{\infty}[\)

Do you have any idea for a workaround?

One idea I had, but coudn't figure it out, was to demand no spaces in between, such that

\(]0,+\infty [\)

would be OK.

@clason
Copy link

clason commented Jul 21, 2024

As LaTeX is whitespace insensitive, the grammar shouldn't be, either.

@jdujava
Copy link
Contributor Author

jdujava commented Jul 21, 2024

That would be ideal, but if there isn't any nice workaround, aren't some tradeoffs necessary? Current parsing of command arguments is rather lacking in my opinion.

@clason
Copy link

clason commented Jul 21, 2024

Yes, and I think this would be the better tradeoff, unfortunate at this is -- unpredictable syntax highlighting (depending on whitespace or not -- and remember that you're not always working with code you've written yourself) would be worse.

@jdujava
Copy link
Contributor Author

jdujava commented Jul 21, 2024

Hmm, understandable. Is there some way to mitigate problems caused by single brackets in math mode? Crate the grammar which always closes the math mode (even if there is such single bracket, which would suggest command argument) or something like that?

@clason
Copy link

clason commented Jul 21, 2024

Probably would need a lot more scanner complexity; I don't see how else to deal with this. (And LaTeX is provably impossible to parse and arguably the worst possible language for LR parsers... So any highlighting is purely best effort and should prioritize the "happy path".)

But @pfoerster is smarter than me and maybe has better ideas ;)

@pfoerster
Copy link
Member

Do you have any idea for a workaround?

One idea would be to specifically allow for unbalanced brackets in generic_command. This does not require changes to the lexer.
Something like this should work (passes tree-sitter test):

image

jdujava added 3 commits July 25, 2024 01:02
- support bracketed arguments
- try to recognize key-value arguments

- TODO: handle single brackets in math mode

Signed-off-by: Jonas Dujava <[email protected]>
Signed-off-by: Jonas Dujava <[email protected]>
@jdujava jdujava force-pushed the improve-generic-command branch from fcc0e8b to c6e4edf Compare July 25, 2024 14:17
@jdujava
Copy link
Contributor Author

jdujava commented Jul 25, 2024

I tried it, but in my limited testing it failed to properly parse something like \(x \in [a,b)\).

For slight modification of your suggestion (see new commit), in the file containing just \(x \in [a,b)\) the ending bracket is magically (I hadn't encountered nothing similar before, can you please explain?) appended in tree-sitter tree

(source_file ; [0, 0] - [1, 0]
  (inline_formula ; [0, 0] - [0, 15]
    "\\(" ; [0, 0] - [0, 2]
    (text ; [0, 2] - [0, 13]
      word: (word) ; [0, 2] - [0, 3]
      word: (generic_command ; [0, 4] - [0, 13]
        command: (command_name) ; [0, 4] - [0, 7]
        arg: (brack_group_generic_arg ; [0, 8] - [0, 13]
          "[" ; [0, 8] - [0, 9]
          (text ; [0, 9] - [0, 10]
            word: (word)) ; [0, 9] - [0, 10]
          "," ; [0, 10] - [0, 11]
          (text ; [0, 11] - [0, 12]
            word: (word)) ; [0, 11] - [0, 12]
          ")" ; [0, 12] - [0, 13]
          "]"))) ; [0, 13] - [0, 13]  <<<<<-------------------- WHAT?
    "\\)")) ; [0, 13] - [0, 15]

but for more complex file it oftentimes produces errors (bracket is interpreted as start of (brack_group_generic_arg), but then it can't find the end and fails on the end of (inline_formula).

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.

Commands with key-value arguments commands with optional arguments do not get parsed correctly
3 participants