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

Issue #141 Matching bracket support with language configuration #166

Merged
merged 1 commit into from
Aug 15, 2018

Conversation

LucasBullen
Copy link

  • Enable bracket matching using the surroundingPairs in the language-configuration.json file
  • KNOWN ISSUE: Does not take in account "back slashed" or in string matched pairs
    • ex: {"}"} will match the first }
    • ex: "\"" will match the first two "

Signed-off-by: Lucas Bullen [email protected]

…ration

 - Enable bracket matching using the surroundingPairs in the
language-configuration.json file

 - KNOWN ISSUE: Does not take in account "back slashed" or in string
matched pairs
  - ex: {"}"} will match the first }
  - ex: "\"" will match the first two '"'

Signed-off-by: Lucas Bullen <[email protected]>
@LucasBullen
Copy link
Author

@angelozerr what are your thoughts on the limitation? Should I assume that " and ' are used for strings and ignore matching brackets in them? Same for \?

@angelozerr
Copy link
Contributor

@LucnasBullen thanks for your work, but IMHO I think we should implement ICharacterPairMatcher to avoid creating new annotation type, etc.

But the hard thing is to support comments for instance (to avoid doing matching bracket inside comments). To do that, the TMModel should be refactored to give the capability to retrieve token from an offset (or compute it, if it's not available). VSCode does that.

The first thing to do is to support #38 but it's a big big task -(

I suggets you to read https://code.visualstudio.com/blogs/2017/02/08/syntax-highlighting-optimizations

@LucasBullen
Copy link
Author

I looked into ICharacterPairMatcher however unless I'm mistaken, surrounding pairs are strings and can be longer than just a character (<<example>>). I've yet to read what you sent so excuse if that has the answer to this problem.

Agree that this needs to deal with the comments situation. I'll look into #38 and your link. Hope to get more language-configuration support in asap as it is a definite necessity.

@angelozerr
Copy link
Contributor

I looked into ICharacterPairMatcher however unless I'm mistaken, surrounding pairs are strings and can be longer than just a character (<>). I've yet to read what you sent so excuse if that has the answer to this problem.

Don't excuse you @LucasBullen, to be honnest with you, it's just some idea that I had, but perhaps Im' wring.

Agree that this needs to deal with the comments situation. I'll look into #38 and your link. Hope to get more language-configuration support in asap as it is a definite necessity.

I agree with you that maching bracket is very important. Perhaps in a first iteration we can use your work? But I think ICharacterPairMatcher should be used (perhaps generic editor should provide an extension for that?)

@LucasBullen
Copy link
Author

Getting lost running around the internals. What is the best way to find out if a TMToken is a comment?
Follow up: The TMModel does not take in account the language-configuration file correct? If so should it be edited to add tokens for surroundingPairs and comments (possibly also folding and autoClosingPairs) from the config file or should these two sets be kept separate?

@angelozerr
Copy link
Contributor

Getting lost running around the internals.

I understand, it's a little hard code -(

What is the best way to find out if a TMToken is a comment?

It's a very good question. I don't remember how to do that, but to be honnest with you I would like to refactor to support #38 to have a better clean token model (which will consume less memory, etc). But it's a very hard task. Once we will this new token model, it will open the door for a lot of things like providing an hover to see the textmate token (for developer of texmate grammar)

Follow up: The TMModel does not take in account the language-configuration file correct?

No.

If so should it be edited to add tokens for surroundingPairs and comments (possibly also folding and autoClosingPairs) from the config file or should these two sets be kept separate?

It should be separate. The basic idea is to manage those features with token models.

In other words I have started to implement #38 (see https://github.com/eclipse/tm4e/blob/master/org.eclipse.tm4e.core/src/main/java/org/eclipse/tm4e/core/internal/grammar/Grammar.java#L195) but now I must consume it https://github.com/eclipse/tm4e/blob/master/org.eclipse.tm4e.core/src/main/java/org/eclipse/tm4e/core/model/Tokenizer.java#L64 but it requires a lot of changes (for theme too). To do that I need to study the vscode code at https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/services/textMate/electron-browser/TMSyntax.ts#L448

I'm not sure that you want to try it, as it's a very big hard task, but I would like to support that to provide a clean token model.

The main problem with token model is that it cannot be ready (if it takes time), so we must see how vscode manage that (when matching bracket need the token model on a line which is not available) to avoid freezing.

So I think we should manage matching bracket at first without "comment" case. I can accept your PR if you wish, but IMHO I think matching bracket should use Eclipse ICharacterPairMatcher. But to consume it, we need a new extension point for generic editor.

@LucasBullen what do you think about that?

@LucasBullen
Copy link
Author

I'm not sure that you want to try it, as it's a very big hard task, but I would like to support that to provide a clean token model.

I won't have the time in the near future to do this as I will only be working full time until the end of next week, then will be back at school and working periodically. I'm looking forward to this feature as I agree it is the best way forward.

but IMHO I think matching bracket should use Eclipse ICharacterPairMatcher. But to consume it, we need a new extension point for generic editor.

We've already had this discussion before and an extension to the generic editor is not required to add ICharacterPairMatcher based bracket matching. However, our problem stems from the Character part as we must support String pairs and the full infrastructure in jface only supports characters.

I agree that a IStringPairMatcher should be added to jface, but to have that implemented and added to tm4e is longer than my semi-selfish 2 week deadline.

@angelozerr
Copy link
Contributor

I understand @LucasBullen. So if I understand you wish to merge your work, is that? i'm OK with that.

@LucasBullen
Copy link
Author

Yes, then I will move to comment toggling, so that we have the majority of language-configuration.json support implemented and ready for upgrading when we have the token support.

@angelozerr angelozerr merged commit 143f2a7 into eclipse-tm4e:master Aug 15, 2018
@angelozerr
Copy link
Contributor

Thank a lot @LucasBullen ! We will see in the future support for comment wityh matching bracket once #38 will be implemented

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.

2 participants