Update links to subcommands #536
Conversation
iAdramelk
commented
Aug 8, 2019
iAdramelk
commented
- Chech if page exists and only create link it if it exists
- If subcommand page exists – link to page, if not, create anchor
- Chech if page exists and only create link it if it exists - If subcommand page exists link to page, if not, create anchor
|
|
||
| if (parts.length > 2) { | ||
| url += '#' + parts[2] | ||
| if (getItemByPath(tmpUrl + parts[2])) { |
There was a problem hiding this comment.
a little bit concerned re the performance of this if page is large enough, and sidebar will become bigger
also, why do we need to do this check - what are the cases when we want to be "smart"?
also, what are the cases with #?
There was a problem hiding this comment.
It's basically just object search in the list with 200 nodes without computations. And we only run it at max twice per node with dvc command. Right now all pages have at max 10 commands, so this search run max 20 times. It should be pretty fast.
But I think that I can change Makrdown.js to PureComponent from Component to prevent rerenders without markdown content changing. This way search will be run only on initial render and on page change.
About why we need it. We basically have this possible cases:
dvc command.dvc command subcommand- subcommand page exists. E. g.dvc metrics add.dvc command subcommand- no subcommand page, but parent page has header with the command name. E. g.dvc config cache(there is a #cache header on the config page).dvc command subcommand- no subcommand page and parent page don't have corresponding header.
getItemByPath is used to check if subcommand page exists or not and if not set # instead. It's possible that there is no such anchor on the page, but unfortunately I can't check it without downloading .md file.
| @@ -1,23 +1,36 @@ | |||
| ;`use strict` | |||
|
|
|||
| import visit from 'unist-util-visit' | |||
| import { getItemByPath } from '../../SidebarMenu/helper' | |||
|
|
|||
| function linker() { | |||
| function transformer(tree) { | |||
| visit(tree, 'inlineCode', function(node, index, parent) { | |||
| if (parent.type !== 'link' && /dvc\s+[a-z-.]+/.test(node.value)) { | |||
There was a problem hiding this comment.
do we need to change the regex here to match three-parts nodes?
There was a problem hiding this comment.
Not necessary. Right now this regexp just check if this is a dvc command or not. Breaking it to parts happens at the next line let parts = node.value.split(/\s+/). We can add additional check to part[2] to check if it matches command syntax, but we can leave regexp itself as is.
|
https://dvc-org-pr-536.herokuapp.com/doc/commands-reference/remote/default - |
There was a problem hiding this comment.
@shcheklein can confirm dvc remote modify bug, will check it.
|
|
||
| if (parts.length > 2) { | ||
| url += '#' + parts[2] | ||
| if (getItemByPath(tmpUrl + parts[2])) { |
There was a problem hiding this comment.
It's basically just object search in the list with 200 nodes without computations. And we only run it at max twice per node with dvc command. Right now all pages have at max 10 commands, so this search run max 20 times. It should be pretty fast.
But I think that I can change Makrdown.js to PureComponent from Component to prevent rerenders without markdown content changing. This way search will be run only on initial render and on page change.
About why we need it. We basically have this possible cases:
dvc command.dvc command subcommand- subcommand page exists. E. g.dvc metrics add.dvc command subcommand- no subcommand page, but parent page has header with the command name. E. g.dvc config cache(there is a #cache header on the config page).dvc command subcommand- no subcommand page and parent page don't have corresponding header.
getItemByPath is used to check if subcommand page exists or not and if not set # instead. It's possible that there is no such anchor on the page, but unfortunately I can't check it without downloading .md file.
