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

Java syntax highlighting wrong (JavaDoc does not end) #160

Closed
sharkdp opened this issue May 8, 2018 · 9 comments · Fixed by #220
Closed

Java syntax highlighting wrong (JavaDoc does not end) #160

sharkdp opened this issue May 8, 2018 · 9 comments · Fixed by #220
Labels

Comments

@sharkdp
Copy link
Contributor

sharkdp commented May 8, 2018

The following Java file

/**
 * test
 */
class ExampleClass {
}

renders correctly with syncat and sublimehq/Packages pinned to fa6b862 (see testdata/Packages):

image

However, when updating sublimehq/Packages to master (currently at 0f852a3) ...

(cd testdata/Packages; git fetch; git checkout master)
make packs

the docstring highlighting does not end:

image

I'm not sure if syntect always aims to support the current state of sublimehq/Packages, but I thought it might be worth to report this.

@sharkdp
Copy link
Contributor Author

sharkdp commented May 8, 2018

Note: this happens with both newlines and no-newlines mode.

@Keats
Copy link
Contributor

Keats commented May 11, 2018

Bisecting shows it come from sublimehq/Packages#1351
It's a rewrite of JavaDoc so not easy to pinpoint the reason, unless @keith-hall can magically figure out the issue from looking at the regexes like he seems to be able to!

Since it keeps treating the rest of the class as JavaDoc, I'm guessing some context is not popped? Related to #104 maybe?

@keith-hall
Copy link
Collaborator

Changing https://github.com/sublimehq/Packages/pull/1351/files#diff-9100242f2b69d8e25efae312969f1256R15 to - match: ^\s*(\*)(?!/)\s*(?!\s*@) seems to fix the problem. My guess would be that somehow syntect is still behaving differently to Sublime Text in regards to prototype and with_prototype contexts, because in Sublime, there are no problems.

@keith-hall
Copy link
Collaborator

keith-hall commented May 20, 2018

Looks like it could be caused by the same problem that plagues the ASP syntax in syntect re with_prototype. I've made a PR to get the JavaDoc syntax to use embed instead which seems to fix the problem in syntect, and will have the bonus of using less RAM in Sublime too :)
EDIT: although I still have to wonder how it helps in syntect, seeing as we convert embed actions to the equivalent with_prototype anyway... it'd be great to identify the underlying bug and resolve that.

@emidoots
Copy link
Contributor

emidoots commented Jun 1, 2018

I wasted a bit of time due to not finding this issue earlier, but hopefully someone else can benefit from what I've discovered here. I've managed to boil down the JavaDoc syntax into this much smaller form which demonstrates the issue:

%YAML 1.2
---
name: Javadoc
scope: text.html.javadoc
contexts:
  prototype:
    - match: ^(\*)
      captures:
        1: punctuation.definition.comment.javadoc

  main:
    - meta_include_prototype: false
    - match: /\*\*
      scope: comment.block.documentation.javadoc punctuation.definition.comment.begin.javadoc
      embed: contents
      embed_scope: comment.block.documentation.javadoc text.html.javadoc
      escape: \*/
      escape_captures:
        0: comment.block.documentation.javadoc punctuation.definition.comment.end.javadoc

  contents:
    - match: ''
      scope: comment.block.documentation.javadoc

I am using the newlines (NOT no newlines) mode with a patched version of synhtml, which produces the following output:


Test case 1

package org.joda.time.chrono;/**
**/class BasicChronology extends AssembledChronology {
}

image


Test case 2

package org.joda.time.chrono;/**
*/class BasicChronology extends AssembledChronology {
}

image


  • Both of these render identical from inside ST3, but not via synhtml (with newlines).
  • The second test case (the one that fails) is using a single */ rather than a double **/ to close the comment.
  • Removing the prototype section has no visible effect in ST3, however in synhtml it appears to solve the issue so it must be related.

I didn't manage to track the issue further down than this, though, as I'm just not proficient enough in Rust or Syntect's inner workings yet.

@trishume
Copy link
Owner

trishume commented Jun 1, 2018

Fixed by #172. Anyone who uses syntect: let me know if you'd like a prompt release for this, otherwise I'll wait and batch it with other things.

@trishume trishume closed this as completed Jun 1, 2018
emidoots added a commit to emidoots/Packages that referenced this issue Jun 9, 2018
@emidoots
Copy link
Contributor

emidoots commented Jun 9, 2018

@trishume could you reopen this issue?

@keith-hall Unfortunately I just discovered that #172 did not fully resolve this.

It did resolve the "test case 2" I wrote above, however if there is a space in front of the final */ it does not resolve the issue. This code:

package org.joda.time.chrono;/**
 */class BasicChronology extends AssembledChronology {
}

Renders via syncat (with the latest trishume/syntect repo, latest sublimehq/Packages repo, and make assets ran) as:

image

Without the space in front of */ it works fine:

image

It renders fine in ST3 with this same syntax version, of course.

I have not yet narrowed down the problem, however I did discover a crude workaround that for some reason solves the issue in Syntect: emidoots/Packages@d8c964e

@keith-hall keith-hall reopened this Jun 9, 2018
@keith-hall
Copy link
Collaborator

another crude workaround that significantly reduces the number of syntest failures for the Java package is adding - meta_include_prototype: false to the reference context, yet it doesn't seem to fix this concrete problem. Sublime Text must do something slightly differently in terms of prototype contexts that syntect isn't quite replicating yet. This would probably be easier to debug if we could get the context names shown (and I guess we could number the anonymous contexts somehow to name those, similar to how Sublime does it) as discussed at #146 (comment), because doing a println!("{:?}", self.stack); produces a lot of hard to read output at the moment.

@Soulsbane
Copy link

This is also occurring in D programming language.
Example:

/**
My comment
*/

Removing the additional asterisk from allows highlighting until another double asterisk is found.
Thanks!

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

Successfully merging a pull request may close this issue.

6 participants