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

Update packages (sublime syntaxes) #228

Open
robinst opened this issue Jan 8, 2019 · 11 comments
Open

Update packages (sublime syntaxes) #228

robinst opened this issue Jan 8, 2019 · 11 comments

Comments

@robinst
Copy link
Collaborator

robinst commented Jan 8, 2019

I'm looking at updating testdata/Packages and syntect's built-in dumps of them.

Just doing a git submodule update --recursive --remote && make packs, the second step fails with:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value:
ParseSyntax(InvalidYaml(ScanError { mark: Marker { index: 9438, line: 256, col: 12 },
info: "while parsing a node, did not find expected node content" }),
Some("testdata/Packages/Go/Go.sublime-syntax"))', libcore/result.rs:1009:5

I suspect this is something that some YAML parsers don't have a problem with, but the one we use for syntect does, or something that used to be allowed with an older YAML spec. I'll raise a PR for https://github.com/sublimehq/Packages/.

robinst added a commit to robinst/Packages that referenced this issue Jan 8, 2019
The YAML 1.2 parser we use in syntect has a problem handling the unquoted syntax, see trishume/syntect#228
@keith-hall
Copy link
Collaborator

see also chyh1990/yaml-rust#118

cmyr added a commit to xi-editor/syntect-resources that referenced this issue Jan 20, 2019
The most recent go syntax definition doesn't work with syntect
(see trishume/syntect#228) so this
replaces it with an older, working version.
cmyr added a commit to xi-editor/syntect-resources that referenced this issue Jan 20, 2019
The most recent go syntax definition doesn't work with syntect
(see trishume/syntect#228) so this
replaces it with an older, working version.
@robinst
Copy link
Collaborator Author

robinst commented Mar 1, 2019

I've raised a PR to fix yaml-rust: chyh1990/yaml-rust#122

Unfortunately it looks like yaml-rust is not actively maintained.

Should we:

  1. Regenerate the packs with the fix but keep the yaml dependency as-is (not sure if possible)
  2. Depend on a fork with the fix until a new version of yaml-rust comes out
  3. Patch the go syntax in our submodule until yaml-rust is fixed

What do people think?

@trishume
Copy link
Owner

trishume commented Mar 1, 2019

Depending on a fork sounds best to me

@robinst
Copy link
Collaborator Author

robinst commented Mar 8, 2019

Good news, my fix was merged and yaml-rust 0.4.3 was released, we don't need to fork :).

But there's another problem, with the Python syntax:

     Running `target/debug/examples/gendata synpack testdata/Packages assets/default_newlines.packdump assets/default_nonewlines.packdump assets/default_metadata.packdump testdata/DefaultPackage`
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseSyntax(RegexCompileError("\\((?=(?x:\n  \\s+                      # whitespace\n  | [urfb]*\"(?:\\\\.|[^\"])*\" # strings\n  | [urfb]*\'(?:\\\\.|[^\'])*\' # ^\n  | [\\d.ej]+               # numerics\n  | [+*/%@-] | // | and | or # operators\n  | (\\b[[:alpha:]_][[:alnum:]_]*\\b *\\. *)*\\b[[:alpha:]_][[:alnum:]_]*\\b               # a path\n)*,|\\s*\\*(\\b[[:alpha:]_][[:alnum:]_]*\\b *\\. *)*\\b[[:alpha:]_][[:alnum:]_]*\\b)", Error(-114, target of repeat operator is invalid)), Some("testdata/Packages/Python/Python.sublime-syntax"))', src/libcore/result.rs:997:5

The regex with better formatting:

\((?=(?x:
  \s+                      # whitespace
  | [urfb]*"(?:\\.|[^"])*" # strings
  | [urfb]*'(?:\\.|[^'])*' # ^
  | [\d.ej]+               # numerics
  | [+*/%@-] | // | and | or # operators
  | (\b[[:alpha:]_][[:alnum:]_]*\b *\. *)*\b[[:alpha:]_][[:alnum:]_]*\b               # a path
)*,|\s*\*(\b[[:alpha:]_][[:alnum:]_]*\b *\. *)*\b[[:alpha:]_][[:alnum:]_]*\b)

The problem is this bit: \b * (note the space between)

Because this is within a (?x:...) where whitespace is ignored, it's equivalent to \b* which is invalid.

Note that the pattern comes from a variable, here: https://github.com/sublimehq/Packages/blob/master/Python/Python.sublime-syntax#L35

So now I'm wondering:

  • Is this just a bug in the syntax?
  • Does Sublime Text, when it includes the regex of a variable, wrap that regex in (?-x:...) to disable extended mode? In that case, we can do the same in syntect.
  • Does the engine that Sublime Text use treat (?x:\b *) differently than onig?

@keith-hall maybe you can help out? :)

@keith-hall
Copy link
Collaborator

I believe it is a bug in the syntax definition and a bug in ST that it "works": sublimehq/sublime_text#2354

@robinst
Copy link
Collaborator Author

robinst commented Mar 8, 2019

Hmm, that looks like a similar problem, but I'm not sure it's the same. In our case, we only have a (?x:...), not a bare (?x)...

@keith-hall
Copy link
Collaborator

Good point. Maybe ST's regex engine ignores the \b* completely rather than reporting it as an error? I'm not at a computer to experiment atm. I'm pretty certain ST doesn't implicitly wrap variables in a (?-x:...), I guess this should be possible to test in ST by trying a variable that relies on extended mode and setting it from where it is referenced, seeing how it matches text, and/or trying an invalid pattern (i.e. missing a closing paren) to see what "expanded" pattern ST shows in the error message.

@robinst
Copy link
Collaborator Author

robinst commented Mar 11, 2019

You were right! So:

  1. \b* does not result in an error in Sublime Text's regex engine

  2. An included pattern is not implicitly wrapped in (?-x: ...), e.g. this:

    variables:
      var: a b c
    contexts:
      main:
        - match: '(?x: {{var}})'
          scope: test.good
    

    Matches abc

  3. If I force Sublime Text to use the Oniguruma engine by appending (?<!0), it results in the same error:

Screen Shot 2019-03-11 at 17 04 03

So that means it's just a bug in the Python syntax, I'll raise a PR for it.

@keith-hall
Copy link
Collaborator

Thanks for taking the time to investigate and solve this @robinst :)

@robinst
Copy link
Collaborator Author

robinst commented Mar 11, 2019

PR: sublimehq/Packages#1897

@robinst
Copy link
Collaborator Author

robinst commented Mar 24, 2019

Raised a PR with the updates here: #246

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

No branches or pull requests

4 participants