Nothing Special   »   [go: up one dir, main page]

Page MenuHomePhabricator

Syntax highlighting: make <ref> tags and <nowiki> tags green
Closed, ResolvedPublic2 Estimated Story Points

Description

Make <ref> tags and <nowiki> tags green in CodeMirror.

Event Timeline

kaldari set the point value for this task to 2.
DannyH triaged this task as Medium priority.Apr 4 2017, 10:52 PM
DannyH moved this task from Needs Discussion to Up Next (June 3-21) on the Community-Tech board.
Pastakhov subscribed.

What about <nowiki> tags?

Change 346499 had a related patch set uploaded (by Pastakhov):
[mediawiki/extensions/CodeMirror@master] Syntax highlighting: make <ref>, <nowiki> tags green

https://gerrit.wikimedia.org/r/346499

DannyH renamed this task from Syntax highlighting: make <ref> tags green to Syntax highlighting: make <ref> tags and <nowiki> tags green.Apr 5 2017, 12:35 PM
DannyH updated the task description. (Show Details)

@Pastakhov Oh, good call. I changed the ticket, thanks!

Sorry, I mixed up <ref> and <pre> tags.
The patch https://gerrit.wikimedia.org/r/#/c/346499 is only for <pre> and <nowiki> tags.

For <ref> tag - patch is https://gerrit.wikimedia.org/r/#/c/348673/

Change 348673 had a related patch set uploaded (by Pastakhov):
[mediawiki/extensions/Cite@master] Highlighting text as wikitext inside <ref> tag for CodeMirror extension

https://gerrit.wikimedia.org/r/348673

@Pastakhov, I amended your first patch to add ref tag support too. Can you review it? We can abandon the other one.

@Niharika, I saw the patch (Patch Set 2), it is incorrect (please take a look at my answer in comments )
Patch https://gerrit.wikimedia.org/r/#/c/346499 makes <pre> and <nowiki> tags green, and also allows extensions to define colors for own tags.
(I was sure by mistake that this task for <pre> tags. I have to be more careful)

As I wrote in comment, extensions should define how to highlight their own tags (by default it is plain text). <ref> tag belongs the Cite extension and I created patch that makes tag green and highlights text inside one. https://gerrit.wikimedia.org/r/348673

@DannyH, should all tags be green? There are HTML tags (like <big>) and extension tags ( like <ref> ) and specific wiki markup tags (like <nowiki>)
Originally HTML tags (and specific wiki markup tags except <pre> and <nowiki>) was green and extension tags was violet. I did this to visually separate HTML tags and extension tags since it look similar but behavior is different.

I'd say we switch all tags to green and see how it works. The thing is, we as developers know that HTML, extension and wiki markup tags are all different but for an editor, it is all the same. They look same, they work in a similar fashion.
If it doesn't look good, we can consider switching back to different colors.

Change 349260 had a related patch set uploaded (by Pastakhov):
[mediawiki/extensions/CodeMirror@master] Syntax highlighting: Switch extension tags to green

https://gerrit.wikimedia.org/r/349260

I think having all tags be green would be fine (and simplify the implementation). Niharika is probably right that most editors aren't going to recognize the difference between the different types of tags. Mostly they just need a way to visually separate them from the content and templates.

It never occurred to me that for someone it might be better that all the tags be the same color :)
I think we should make several color themes (T163533) in the future and editors will be able to choose the best for themselves.

@Pastakhov, @Niharika: We still need to make some distinction in the color coding between tags that are parsed and tags that aren't parsed (i.e. valid and invalid tags). Tags that aren't parsed should just stay black (as they do now). Instead of registering each valid tag with CodeMirror from separate extensions, which will be a maintenance headache, I wonder if we could just output the list of valid tags to the client (since MediaWiki already knows this information). We could get the list of valid non-HTML tags from Parser::$mTagHooks and the list of allowed HTML tags from Santizer::getRecognizedTagData(), instead of having CodeMirror rely its own lists. (Community Tech can help implement these core changes if needed.)

These 2 sets of tags could have 2 different classes, as they do now (cm-mw-exttag and cm-mw-htmltag), but color both of them green by default. Personally, I don't like the idea of extensions setting their own custom colors for different tags. This will inevitably lead to color conflicts and bad color choices. But I do like Pastakhov's idea of having different color themes available.

I wonder if we could just output the list of valid tags to the client

@kaldari, CodeMirror already gets the list of valid non-HTML tags from Parser.

and the list of allowed HTML tags from Santizer::getRecognizedTagData()

The list of valid HTML tags was copied from Sanitizer class to CedeMirror since this list is static.

Instead of registering each valid tag with CodeMirror from separate extensions

CodeMirror integration (T163238) needed only for highlighting text (or maybe errors in the syntax) inside extension tags, because extensions themselves parse text inside the tags.

I don't like the idea of extensions setting their own custom colors for different tags

It's just an opportunity to do it. I agree this can lead to color conflicts but only if someone does it on purpose. I do not call to colorize all extension tags in different colors, I think that in some special cases, the ability to highlight extension tags in different color can be useful.

Patch https://gerrit.wikimedia.org/r/349260 changes default color of non-HTML tags to green (nothing more)
Patch https://gerrit.wikimedia.org/r/346499 adds an opportunity to use different styles for different extension tags (nothing more)
Patch https://gerrit.wikimedia.org/r/348673 says to CodeMirror that text inside <ref> tags should be highlighted as wikitext markup (nothing more).

Let's merge first and second patches, and next step will be T162234 where I put in order CSS.

Change 349260 merged by jenkins-bot:
[mediawiki/extensions/CodeMirror@master] Syntax highlighting: Switch extension tags to green

https://gerrit.wikimedia.org/r/349260

@Pastakhov: Thanks for the explanation. Now it all makes sense to me. I think 349260 + 346499 + 348673 seems like a sensible solution.

@Pastakhov Unrelated question - are there any plans to highlight code embedded in wiki pages as well or you'd want the SyntaxHighlight extension to handle that?

@Niharika The CodeMirror and SyntaxHighlight solve different tasks in different ways and one can't replace the other. We can make the text in the editor look the same as on the page (I mean the text or code inside <source> tags)

@Niharika The CodeMirror and SyntaxHighlight solve different tasks in different ways and one can't replace the other. We can make the text in the editor look the same as on the page (I mean the text or code inside <source> tags)

I know that. I was asking if we need to do the code for this inside CodeMirror or SyntaxHighlight (using the CodeMirror hook).

I'm sure we should do the code inside SyntaxHighlight