Dvc.org: 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

[jorgeorpinel]

Hi. Found another issue with enabling automatic links in headers (see https://github.com/iterative/dvc.org/pull/1735#pullrequestreview-478009965): the text disappears in headers of expandable <details> sections.

Bumping the priority a little. Let's fix this bug at least? Just in case, even if we opt for not using back ticks in headers.

[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.

[rogermparent]

Actually, it seems that change didn't fix the issue. It may be a deeper interaction between the code quotes and the details tag.

[jorgeorpinel]

auto-links in the description box (or even regular headers) looks a bit out-of-place in my opinion

Agree.

that change didn't fix the issue

What change? 🙂

[rogermparent]

What change? slightly_smiling_face

Ah, I posted that before the PR. Simply removing the auto-linker from headers didn't fix the details issue, and details had to be changed to actually fix it.

[jorgeorpinel]

OK so the <details> part is addressed by #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.

[jorgeorpinel]

Hi! Me again. It seems now dvc by itself doesn't link to /doc/command-reference anymore. As usual Idk when this change happened but it would be great to reinstate that behavior 🙂

I chose to comment here instead of opening another issue but it could be separate of course.

UPDATE: Actually, you know what? Never mind. It's better this way! Sometimes we use dvc in other context, like to refer to the Python package name.