[WIP] css: highlight subcommands for dvc

[algomaster99]

Fixes #541

[shcheklein]

@algomaster99 this is not about usage code block only, this is about dvc as well

[jorgeorpinel]

This is great so far! Fixes a problem we didn't even notice in the ```usage blocks 🙂

But #541 is about dvc blocks actually, can you also fix that? We should merge this regardless but if you can include that too, even better.

Thanks @algomaster99!

[algomaster99]

Don't merge this yet. I know it is becoming bold because of the className: 'strong' added in dvc.js. But how exactly are the DVC commands becoming blue (docco) in colour? Where have we defined this?
I got this. This line does it - const dvcStyle = Object.assign({}, docco)

1. What are the valid classNames here?
2. I think it has something to with lexemes. Can anyone explain it? @shcheklein

[jorgeorpinel]

This seems to be fixed form looking at the review app (e.g. https://dvc-org-pr-713.herokuapp.com/doc/command-reference/pipeline/list). Is this still WIP (work in progress) @algomaster99?

p.s. more info on lexemes: https://en.wikipedia.org/wiki/Lexical_analysis and https://highlightjs.readthedocs.io/en/latest/language-guide.html in our specific case.

[algomaster99]

@jorgeorpinel So I have written the regex for the third subcommand as /\w+(?![\S])/ which is looks for characters other than special characters and matches them. This helped to ignore highlighting [email protected] ... in dvc import [email protected] ...

But it will match pics in dvc add pics or dvc add foo. This problem can have two solutions:-

1. Either we write directories in commands as ./pics and files as foo.txt.
2. We hardcode the regex in third sub-mode like this:-

contains: [
                  {
                    begin: /(list|show|modify|dir|default|add|remove)/,
                    keywords: {
                      built_in: 'list show modify dir default add remove'
                    }
                  }
                ],

I prefer the first solution. What do you say?

[shcheklein]

@algomaster99

Either we write directories in commands as ./pics and files as foo.txt

This would complicate the docs artificially. Highlighter should be helping, not creating some additional limitations.

We hardcode the regex in third sub-mode like this:

It might start highlighting all there keywords somewhere in the middle, not only after dvc [command]?

[shcheklein]

see my comment above

[algomaster99]

@shcheklein I think it will match only till second sub-command. This behaviour might exist becuase higlight.js considers spaces as delimiters.

Changes in regex

Output:

[shcheklein]

@algomaster99 I was referring to the current implementation. It's clearly broken since it highlights dvc add pics for example.

Could you explain me how does it work? begin: /^\s*\$\s(dvc|git) [a-z-]+/ supposed to math only the dvc something or git something and stop right after that. The whole thing should not be matching anything after that unless I'm missing something.

[algomaster99]

@shcheklein yeah sure!
Check the changes in lang/dvc.js. I am basically creating another sub mode after the first sub-command. I changed to /dvc [a-z-]+/ to /dvc [a-z-]+ ?/ which starts checking for yet another sub command. Then I have started another sub-mode using contains which matches yet another word.

I think your confusion might be why pics in dvc add pics wasn't highlighted earlier? This is because /dvc [a-z-]+/ matched only till add and ended because it saw a delimiter - (a space) in this case.

[shcheklein]

@algomaster99 I think I understand what you just explained. I don't get why. I thought that begin: /^\s*\$\s(dvc|git) [a-z-]+/, means that it should consume everything up to the first space after dvc some-thing or git some-thing and stop right after that. /dvc [a-z-]+ ?/ is a sub mode of the parent mode and should not affect (expand in this case) the way we match the parent, unless I'm missing something.

[algomaster99]

@shcheklein maybe it keeps traversing down the nest. If we are thinking like that, it is weird that the second sub command is returned as match.

[shcheklein]

If we are thinking like that, it is weird that the second sub command is returned as match.

exactly. I'm thinking like that by reading the docs. That's why I'm asking you. I don't understand why matching an extra space makes it go and math and extra keyword.

If we don't understand this, it's not a good idea to merge this - someone will have to support/update it later, or it might break after highlighter update.

Could you ask on the highlighter community to clarify this?

[algomaster99]

@shcheklein I moved it to StackOverflow: https://stackoverflow.com/questions/58511529/highlight-js-not-respecting-parent-regex-of-a-sub-mode.

[shcheklein]

@algomaster99 is there a forum/chat/mail list for the highlighter? may be we can ask there?

[algomaster99]

@shcheklein they have a Google Group but it seems very inactive.
We may ask here too https://meta.discourse.org/t/integrating-a-customized-highlight-js-into-my-forum/48758. Although I am not near my computer right now. If you can do it, please do. Otherwise I will do it in a few hours. :)

[shcheklein]

@algomaster99 any progress on this? do you need any help?

[algomaster99]

@shcheklein I am just waiting for the stack overflow response but I plan to look into it tomorrow. I didn't have time today. 😅

[shcheklein]

@algomaster99 why don't we ask this question/file a bug report to the highlight js Github if it's relevant to this?

[algomaster99]

@shcheklein we can try but their GitHub repo was very inactive.

[shcheklein]

@algomaster99 can you share the link? how do you see that it's not active?

[algomaster99]

@shcheklein okay I just saw again. They have committed a few hours ago. Shall I open the issue there?

[shcheklein]

@algomaster99 yes, I think it would reasonable. I think the ticket on SO was too niche to find any help, even bounty does not help it looks like.

[algomaster99]

@shcheklein highlightjs/highlight.js#2238 do you think it need more details. I think it explains well.

[shcheklein]

@algomaster99 it looks good to me, but I'm biased. I hope they will follow up with some questions if they need more info.

[shcheklein]

@algomaster99 really great discussion with Josh in that ticket! Apparently I didn't understand the docs right - relationship between begin/end. I hope you have better idea how to implement it. We can probably add a function that populates sub-rules for pipeline, metrics, etc. The way Josh described.

[algomaster99]

@shcheklein I will need to see how I can implement add_command. Current hack in regex would be to simply ignore pics if this issue has to be fixed urgently otherwise let me look into this more.

[shcheklein]

@algomaster99 hey, any updates on this?

[algomaster99]

@shcheklein no, not yet. Still working on the add function. 😕

[shcheklein]

@algomaster99 I know that you are busy with the conference stuff, but any plans to get this done after that? Or should we reassign and close this PR?

[algomaster99]

@shcheklein no need to close it. I will work on it after the conference. :)

[shcheklein]

@algomaster99 closing this as a stale one, so that someone else can take it. If you at some decide to get it done, please feel free to push new commits and reopen.

[algomaster99]

@shcheklein sure 👍

[algomaster99]

@jorgeorpinel @shcheklein Hey! Saw that you guys finally fixed this problem. Can you tell me which file contains the patch? Apparently a lot of things have changed in the codebase. 😅
Btw, great work shifting to TypeScript :)

[shcheklein]

@algomaster99 we migrated to Gatsby and prisma.js, so it's completely different now and not related even to the work we had done in this PR

[algomaster99]

@shcheklein Oh okay. But where is this lexer written? How is the syntax highlighting handled now?