Description
Angelo ZERR
2017-08-24 11:47:32 EDT
If GenericEditor could provides this feature, it should be nice to provide a "Toggle Highlight" button like lsp4e provides. In this case I think "Toggle Highlight" from lsp4e should be moved to GenericEditor Platform Text and provides an extension point to override it with lsp4e higlighter language server. New Gerrit change created: https://git.eclipse.org/r/106222 New Gerrit change created: https://git.eclipse.org/r/106229 Gerrit change https://git.eclipse.org/r/106222 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=aaab23b17c73ac1bdd89b5c499e158b9c3275fec thanks for the patch Lucas. Can you please add a reference to the new extension point in the N&N and in the documentation? New Gerrit change created: https://git.eclipse.org/r/111165 Gerrit change https://git.eclipse.org/r/111165 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.common.git/commit/?id=54fab19c6a21f019163d0a857c705a4aca916db4 New Gerrit change created: https://git.eclipse.org/r/111167 Gerrit change https://git.eclipse.org/r/111167 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=9a406084b37b695fb29cda328953ca521c8266ab A (non-blocking) issue with the current patch is that the Toggle Highlight command uses a preference that extenders are supposed to consume. It would be more consistent with the idea of the generic editor to have the preference controlling installation/uninstallation of the highlight reconciliers and removal of the related annotations. It would be nice if we could improve that as part of this bug. Note that this is a smell that the highlight reconcilier and annotations in general (in JFace, not in generic editor in particular) would be worth being made an abstract subclass of reconciliers providing more specific actions and settings. But that would be for another iteration. To move over from our discussion on the N&N commit: Yes we could do that, however when disabling the toggle I would like to remove any annotations the user has added. To do this we would have to know what type of annotation has been added, which would require them to use our defined annotation so we can refer to it, which restricts them from having multiple types of annotations or their own style of annotation. (In reply to Lucas Bullen from comment #11) > To do this we would have to know > what type of annotation has been added, which would require them to use our > defined annotation so we can refer to it, which restricts them from having > multiple types of annotations or their own style of annotation. That's the key point. Similarly to common commands or other stuff, we probably need a place in Platform to define a "highlight" annotation type and be able to interact with it. The purpose of the generic editor is more to provide consistency and simplicity to extend a code editor over giving full freedom to extenders (as most of Platform Text already does), so I think defining an annotation type and asking extenders to use it is closer to the generic editor goals. In any case, people who want to use another annotation could still use one, it would only be a recommendation to consistently use a common one for highlight. Thanks a lot Lucas for those good patches! Further improvements regarding definition of the annotation will be covered by bug 527007 Thanks a lot Lucas for your great wor! I have just few comments. Compare to VSCode, it should be cool: * highlight every time the token even if there is just one token. * token should contains '_' like 'indent_size'. Today the token is 'indent'. Anyway thanks for this very cool work! (In reply to Angelo ZERR from comment #14) > Thanks a lot Lucas for your great wor! > > I have just few comments. Compare to VSCode, it should be cool: > > * highlight every time the token even if there is just one token. > * token should contains '_' like 'indent_size'. Today the token is > 'indent'. > > Anyway thanks for this very cool work! Can you please report new bugs for those items and link them to this one? Gerrit change https://git.eclipse.org/r/106229 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=4e9bd88ff6da2d59ca521a78e1ee397cc1ebcfd8 |