highlightjs / highlight.js

JavaScript syntax highlighter with language auto-detection and zero dependencies.
https://highlightjs.org/
BSD 3-Clause "New" or "Revised" License
23.74k stars 3.6k forks source link

C# highlighter breaks on generic base classes #684

Closed damirarh closed 9 years ago

damirarh commented 9 years ago

The following snippet doesn't get highlighted:

public class IncrementalLoadingBehavior : Behavior<LongListSelector>
{ }

If the base class isn't generic, it works as expected:

public class IncrementalLoadingBehavior : Behavior
{ }
Sannis commented 9 years ago

Can you check this with latest pull request?

damirarh commented 9 years ago

You probably meant #683? The above mentioned issue seems to be fixed. The Behavior<LongListSelector> part is not syntax highlighted, but the rest looks fine.

I've encountered an even bigger problem, though. The following block of code brings it into an endless loop that I could only stop by killing the node process:

public void CompareDtoAndBusinessClassesBefore()
{
}

If I replace void with class, it works fine:

public class CompareDtoAndBusinessClassesBefore()
{
}

Also docpad run site generation takes ~25 s after the change and took ~15 s before it (with highlight.js 8.3.0 which is currently referenced from docpad-plugin-highlightjs).

citizenmatt commented 9 years ago

Is this endless loop with the PR from #683 applied?

damirarh commented 9 years ago

Yes, the endless loop issue comes up after #683 is applied. Without it, it works fine.

citizenmatt commented 9 years ago

Hmm. That code snippet works fine in the developer tool, but I've got a snippet that also causes problems. However, it's not an infinite loop, it's just MASSIVELY slow. Here's the snippet:

var cleanupProfile = BulkCodeCleanupActionBuilderBase.CreateProfile(profile =>
  {
    profile.SetSetting(ReplaceByVar.USE_VAR_DESCRIPTOR
  }, new ReplaceByVar.Options()
  {
    BehavourStyle = ReplaceByVar.BehavourStyle.CAN_CHANGE_TO_EXPLICIT,
    ForeachVariableStyle = ReplaceByVar.ForeachVariableStyle.ALWAYS_EXPLICIT,
    LocalVariableStyle = ReplaceByVar.LocalVariableStyle.ALWAYS_EXPLICIT
  }));

var projectFile = myProvider.SourceFile.ToProjectFile();
Assertion.AssertNotNull(projectFile, "projectFile must not be null");

var builder = BulkCodeCleanupContextActionBuilder.CreateByPsiLanguage<CSharpLanguage>(cleanupProfile,
  "Use explicit type everywhere", projectFile.GetSolution(), this);
return builder.CreateBulkActions(projectFile, IntentionsAnchors.ContextActionsAnchor,
  IntentionsAnchors.ContextActionsAnchorPosition);

It gets stuck around the string literal in the assertion. Sits at the top of this loop:

while (true) {
  top.terminators.lastIndex = index;
  match = top.terminators.exec(value);
  if (!match)
    break;
  count = processLexeme(value.substr(index, match.index - index), match[0]);
  index = match.index + count;
}

I'm at a bit of a loss as to why this is happening...

citizenmatt commented 9 years ago

Um. Here's the regex it's trying to evaluate. It's a bit, er, big.

citizenmatt commented 9 years ago

Actually, the size of the regexes isn't the problem. There were a couple of rogue quantifiers that were zero or many instead of zero or one. This has fixed my major near-infinite loops. Can you try the latest version of the PR and see how yours goes?

damirarh commented 9 years ago

The latest version performs much better. None of my code blocks caused any problems. The build time of my complete site is only around 5% slower than with version 8.3.0. It seems you have successfully fixed the issue.

Sannis commented 9 years ago

We can include few benchmarks into test suite to avoid such degradations in future, but I have no ideas how to perfectly automate this now.

isagalaev commented 9 years ago

Okay, if this is indeed fixed by #683 I'm going to close the issue as there's nothing more to do. Feel free to reopen it if I'm missing something!