Open core-ai-bot opened 3 years ago
Comment by petetnt Friday Oct 23, 2015 at 08:59 GMT
Arrow (=>)
syntax is correctly highlighted as .cm-operator
for me, it was added with #11528 (I think, that version contained tons of ES6 improvements).
Comment by abose Friday Oct 23, 2015 at 09:02 GMT
=>
would be better colored in blue[or the color of function
keyword].
Comment by AdriVanHoudt Friday Oct 23, 2015 at 09:04 GMT
The thing is stuff like >= is also .cm-operator right now which is different than an actual fat arrow ofc
Like@
abose said it would be better to color it the same as function (blue in default(?), green for me ;))
Comment by petetnt Friday Oct 23, 2015 at 09:11 GMT
Actually yeah, now that I think about it they should be .cm-keyword
as =>
is not an operator at all, confused the naming with C-like arrow operators ->
for a while.
Comment by abose Friday Oct 23, 2015 at 09:15 GMT
codemirror/CodeMirror#3614 says, the coloring is correct as =>
is correctly colored as a CM operator- which makes sense in one way. Not entirely sure now if we should have a special treatment of an operator.
Comment by abose Friday Oct 23, 2015 at 09:30 GMT
http://www.ecma-international.org/ecma-262/6.0/ does not define =>
as an operator. It is identified as Arrow Function Definitions
.
Comment by abose Friday Oct 23, 2015 at 09:50 GMT
Tagging@
MarcelGerber
=>
is grouped under classes and functions
in es docs. I still think the right color is that of functions as specified in the docs.
Comment by AdriVanHoudt Friday Oct 23, 2015 at 10:19 GMT
@
abose is this possible atm or does codemirror need to do something?
Also cc https://github.com/adobe/brackets/issues/11644
Comment by abose Tuesday Dec 08, 2015 at 12:00 GMT
@
petetnt@
zaggino@
MarcelGerber
Is this something that we should do in the brackets side/code mirror side/ Not do at all.
I feel that =>
should be colored as function definition color as =>
is not specified as an operator rather a function def in http://www.ecma-international.org/ecma-262/6.0/
Comment by lkcampbell Tuesday Dec 08, 2015 at 15:03 GMT
@
abose I think the problem is the fat arrow is a function from a semantics standpoint, but from a syntax standpoint it is used like an infix operator. So, should syntax-coloring be based on semantics or on syntax? There are arguments for both sides, which usually signals a user preference change rather than a core change.
Regardless, not sure how you would easily implement either one. You are talking about customizing the CodeMirror Javascript mode, either directly in the file (which would change the core library code) or on the fly (no idea how to do that) so that a specific set of parsed characters returns a different style string.
Comment by lkcampbell Tuesday Dec 08, 2015 at 15:34 GMT
@
abose Actually, I take part of my previous comment back. I should probably drink my coffee before making early morning comments.
Changing a specific set of parsed characters to return a different string on the fly is exactly what a CodeMirror Overlay does although I haven't tested it yet. It could be done with an extension, so it wouldn't affect core code. Granted, it is a very specific extension :). I can at least look into it and see if it is possible.
Comment by AdriVanHoudt Tuesday Dec 08, 2015 at 15:44 GMT
Should the coloring not be based on the context, I get that looking at purely => it can be 2 things but in a context it will always be 1 no?
Comment by lkcampbell Tuesday Dec 08, 2015 at 16:01 GMT
@
AdriVanHoudt As I mentioned before, arguments on both sides make sense. That means it's a good user preference and/or an extension idea. I am looking into an extension solution for you.
Comment by abose Wednesday Dec 09, 2015 at 08:10 GMT
+1
I am for =>
to be colored differently. But as you said it may be a personal choice. It will add better readability, like it will be eazy to locate arrow functions in code.
Comment by abose Wednesday Dec 09, 2015 at 08:12 GMT
Thanks@
lkcampbell .
Once the PR lands, i guess more people will comment their opinions there :)
Comment by lkcampbell Wednesday Dec 09, 2015 at 14:52 GMT
@
abose My first solution is actually going to be an extension, not a PR, but we can keep this issue open to track the extension to completion. I suppose it could eventually be incorporated into the core code but it doesn't have to be incorporated.
Comment by zaggino Wednesday Dec 09, 2015 at 20:37 GMT
Did anyone reach out to CodeMirror repo for at least a comment on this before we start hacking it?
Comment by petetnt Wednesday Dec 09, 2015 at 20:38 GMT
@
zaggino see https://github.com/codemirror/CodeMirror/issues/3614
Comment by zaggino Wednesday Dec 09, 2015 at 21:19 GMT
Ah right, thanks, didn't read that much to the top, my bad.
Comment by lkcampbell Wednesday Dec 09, 2015 at 22:54 GMT
@
AdriVanHoudt@
abose The extension is done and registered.
Go to the extension manager and look for Style Fat Arrows
.
There is one known issue with modes that have embedded or mixed javascript that I haven't been able to fix, but overall, it seems to work fairly well.
Comment by lkcampbell Wednesday Dec 09, 2015 at 23:07 GMT
@
abose I saw you labelled this as Release 1.6
but I don't think you guys will want this solution in the core code. I suggest labelling it as Extension Available
and closing it as such. Anyone who wants this change can use the extension.
Comment by abose Thursday Dec 10, 2015 at 06:02 GMT
Marked it as needs review for other opinions before closing. NB: +1 for this in core :)
Comment by AdriVanHoudt Thursday Dec 10, 2015 at 08:16 GMT
@
lkcampbell installed and seems to work fine! Thanks a lot! :clap:
I am also still +1 for having this is core though
Comment by lkcampbell Thursday Dec 10, 2015 at 15:36 GMT
@
abose you should know about the bug then. This code also changes the style color of fat arrows in non-Javascript code in mixed Javascript files. Open an HTML file with Javascript in it and type a fat arrow in the HTML part of the file, and you will see what I mean.
If there is a way to fix this bug, I don't know how to do it. Not sure how to figure out whether you are currently in Javascript mode or another mode when you are parsing a mixed file using a CodeMirror Overlay.
Comment by lkcampbell Friday Mar 31, 2017 at 03:37 GMT
@
AdriVanHoudt , I am really glad that my simple extension helped you out. It has received a surprisingly high number of downloads for a one-off. More power to you and enjoy the magic that is Open source!!
@
abose, why is this bug still open? I solved it with an extension.
Let's have some closure. I am closing it and removing the priority labels.
Issue by AdriVanHoudt Friday Oct 23, 2015 at 07:55 GMT Originally opened as https://github.com/adobe/brackets/issues/11848
Right now it is interpreted as equal or bigger than. But this should be possible see https://github.com/codemirror/CodeMirror/issues/3614