Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Split the code doing highlighting from the rest. #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickolay
Copy link

@nickolay nickolay commented Mar 8, 2016

Currently highlights.coffee mixes distinct functionality:

  • finding, loading, and selecting grammars to use via first-mate module. This has many dependencies and is a large part of the whole module.
  • returning the HTML for highlighting, given tokens from first-mate's grammar. It is self-contained and pretty straightforward, when you know what it does.

I suggest splitting the two for the sake of those assessing the library.

My story: I was looking for a library to perform syntax highlighting based on AST. I did not find it yet, but found this library.
I got confused by the "Loading Grammars From Modules" section of the README, as it seemed to suggest there was a way to write a JS module to perform the syntax highlighting, but provided no information on the API of such module, so I went looking into the source code to find what kind of input the highlighting code expects from the tokenizer.

(Turns out you only support TextMate-style language definitions, which are handled by first-mate, but the bit of code I'm splitting into a separate file would still be reusable in my case.)

@bcoe
Copy link
Contributor

bcoe commented Aug 12, 2016

@nickolay mind rebasing with master, also I'd love to know @kevinsawicki's thoughts on this refactor.

@nickolay
Copy link
Author

@bcoe Thanks for getting back to me! Sure, if you decide this split-up is something you want, I can prepare an updated PR.

@bcoe
Copy link
Contributor

bcoe commented Jul 8, 2017

@nickolay if you're still interested in landing this refactor, please go ahead and update this pull request and I'll work on getting it landed.

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

Successfully merging this pull request may close these issues.

2 participants