links: automatic command links are too lax now?

[jorgeorpinel]

Should dvc {cmd} {arg} always be linked? E.g.:

From https://dvc.org/doc/command-reference/repro#examples

I have the impression this was not the case before. It used to only do that for commands with subcommands, and perhaps some valid options of dvc config.

[jorgeorpinel]

Cc @rogermparent (let's see what Ivan says though).

[rogermparent]

I don't know when this was introduced, but I'm pretty sure it's not the simpleLinker I added. Its current settings only perform exact string matches. This is looks like behavior from commandLinker that theoretically has existed for a while.

I can change the behavior now that I'm familiar with the mechanism, but I'll need some specific rules.

You can see in the source that commandLinker still links commands even if the subcommand isn't valid.

if (isSubcommandPageExists) {
  url = `${COMMAND_ROOT}${parts[1]}/${parts[2]}`
} else if (isCommandPageExists && hasThirdSegment) {
  url = `${COMMAND_ROOT}${parts[1]}#${parts[2]}`
} else if (isCommandPageExists) {
  url = `${COMMAND_ROOT}${parts[1]}`
}

[shcheklein]

Yeah, I think that was always the case.

Do we want this? I would keep it. May be it should be smart enough to allow only proper third level sub commands and dvc dag <tag> should link to the dag, not to dag#tag

[rogermparent]

Do we want this? I would keep it. May be it should be smart enough to allow only proper third level sub commands and dvc dag <tag> should link to the dag, not to dag#tag

This is the current behavior as hasThirdSegment is only true when the third segment matches a regex determining that it's a proper command. Here's more context around that same snippet:

const DVC_REGEXP = /dvc\s+[a-z][a-z-.]*/
const COMMAND_REGEXP = /^[a-z][a-z-]*$/
const COMMAND_ROOT = '/doc/command-reference/'

module.exports = astNode => {
  const node = astNode[0]
  const parent = astNode[2]

  if (parent.type !== 'link' && DVC_REGEXP.test(node.value)) {
    const parts = node.value.split(/\s+/)
    let url

    const hasThirdSegment = parts[2] && COMMAND_REGEXP.test(parts[2])
    const isCommandPageExists = getItemByPath(`${COMMAND_ROOT}${parts[1]}`)
    const isSubcommandPageExists =
      isCommandPageExists &&
      hasThirdSegment &&
      getItemByPath(`${COMMAND_ROOT}${parts[1]}/${parts[2]}`)

    if (isSubcommandPageExists) {
      url = `${COMMAND_ROOT}${parts[1]}/${parts[2]}`
    } else if (isCommandPageExists && hasThirdSegment) {
      url = `${COMMAND_ROOT}${parts[1]}#${parts[2]}`
    } else if (isCommandPageExists) {
      url = `${COMMAND_ROOT}${parts[1]}`
    }

    createLinkNode(url, astNode)
  }

  return astNode
}

For example, the screenshotted link links to dag, not dag#target, despite <target> being in the link content.

[jorgeorpinel]

I think that was always the case.
Do we want this? I would keep it.

I don't know... I think we want to be able to have short in-line examples that don't get linked like here:

From https://dvc.org/doc/command-reference/pull

^ I don't think that was linked before, BTW. It makes the paragraph have too many links, and it makes the example hard to copy for pasting. And there's plenty similar examples (commit has several, fetch has dvc repro train.dvc at the bottom, etc.)

May be it should be smart enough to allow only proper third level sub commands

Fortunately it doesn't add the # anchor unless the next argument is recognized, it seems. So at least it's not a bug.

[jorgeorpinel]

Fortunately it doesn't add the # anchor unless the next argument is recognized, it seems. So at least it's not a bug.

UPDATE: On that note... I found an inconsistent behavior of this in https://dvc.org/doc/command-reference/fetch:

This one links to https://dvc.org/doc/command-reference/config#cache.

But this one links to just https://dvc.org/doc/command-reference/config.

[jorgeorpinel]

Hi! Also I noticed in some WIP PR that dvc {cmd} will link to the cmd ref even from # md headers (H1, etc.) — I have the impression this also wasn't the case before. Regardless, do we want this behavior? Probably no instances of this in the docs though.

[shcheklein]

@jorgeorpinel yes, I would keep it. Why not? And it's useful in the blog - especially gems.

[jorgeorpinel]

Why not?

Because it seems unexpected to see links in part of a header. Seems to me like too much hyperlinking. The links no doubt will be repeated in the text below 99% of the times. When you click on headers I think you normally expect an anchor that you can copy in the URL bar.

And it's useful in the blog - especially gems.

I think having the option to make regular [md](links) in the headers is good but not sure about the auto links. For example:

From https://dvc.org/blog/june-20-community-gems

.dvc is in 2 different links in the header, and again in the paragraph below. Too much, IMO — not what we intended when writing that MD, probably

[rogermparent]

I can certainly make simpleLinker not apply within headers, like we do with code quotes within explicit links. Even if the linked interaction did work, having auto-links in the description box (or even regular headers) looks a bit out-of-place in my opinion.

[jorgeorpinel]

OK so the <details> part is addressed by iterative/dvc.org#1736, right? Kk, thanks.

[shcheklein]

I think all headers are not auto-linked now. Let's try to keep this way since we did this in PR and see how it goes. Looks like nobody likes auto-linked headers.

[julieg18]

Do we still want to work towards the command linking being less lax or we good to keep it as is?

[jorgeorpinel]

I still think we should re-think this. I think that originally only base commands (and subcommands) e.g. dvc pull, dvc exp show would be linked, not if they came with any arguments. Then we found a case for dvc config <section> to be autolinked into the specific headers under https://dvc.org/doc/command-reference/config#configuration-sections (that still works BTW).

I'm not sure now why/when everything else became autolinked but most often when we have longer code blocks (e.g. dvc push data/something -q) they're actual usage examples in-line in some paragraph (no fenced block needed), or other uses where no link is needed.

In summary linking everything seems overkill to me. If there are specific cases where it's currently useful, we can rewrite those passages to manual links.

[julieg18]

In summary linking everything seems overkill to me. If there are specific cases where it's currently useful, we can rewrite those passages to manual links.

Makes sense. Having too many links could be decreasing readability and making it harder to tab through a page. Screen readers can have difficulty reading through a lot of links on a page as well 🤔 @iterative/websites, what do you think?

[jorgeorpinel]

Rel. iterative/cml.dev#424