Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interactive synopsis Fix #2024 #3140

Merged
merged 29 commits into from
Jan 11, 2022
Merged

Interactive synopsis Fix #2024 #3140

merged 29 commits into from
Jan 11, 2022

Conversation

yathomasi
Copy link
Contributor

@yathomasi yathomasi commented Dec 29, 2021

Feature mentioned on #2024 .

Needs:

  • Special place/way to specify command arguments in pages of https://dvc.org/doc/command-reference
  • link to them from the synopsis? (as well as to -- options)
  • Have anchors for each argument and option

We may not actually need the different/special way to specify command arguments because current enclosing with enclosing brackets should be fine. But, we need to define some set of rules/guidelines so that we are consistent throughout the documentation and also it's would be great to define the parser.

  • define some guidelines for arguments and options(no need as it's defined by argsparse on similar pattern)
  • parse synopsis code block and add anchor # tags to the arguments
  • make sure change in Code component doesn't affect existing syntax highlighting
  • parse options and add respective id for anchor tags

@iterative/websites @shcheklein @iterative/docs


After merge:

  • Change isolated option mentions throughout important docs so they auto-link e.g. from "...use the --out option of dvc import," to just "use dvc import --out."

@shcheklein shcheklein temporarily deployed to dvc-org-interactive-syn-idok0e December 29, 2021 07:57 Inactive
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work so far!

tsconfig.json Show resolved Hide resolved
src/utils/index.ts Outdated Show resolved Hide resolved
src/components/Code/index.tsx Outdated Show resolved Hide resolved
@julieg18
Copy link
Contributor

julieg18 commented Dec 29, 2021

But, we need to define some set of rules/guidelines so that we are consistent throughout the documentation and also it's would be great to define the parser.

One idea that comes to mind is having our /command-reference/RANDOM_COMMAND#options section setup like our glossary page, with each item in the list having a link attached to it. The links in the Synopsis section would take you the desired list item in the options section.

@yathomasi
Copy link
Contributor Author

One idea that comes to mind is having our /command-reference/RANDOM_COMMAND#options section setup like our glossary page, with each item in the list having a link attached to it. The links in the Synopsis section would take you the desired list item in the options section.

Yeah, that's the idea. But, I am having a little bit issue with adding the HTML id attribute to the markdown AST.

@julieg18
Copy link
Contributor

Yeah, that's the idea. But, I am having a little bit issue with adding the HTML id attribute to the markdown AST.

Feel free to message me in Slack if you want to discuss possible solutions!

@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-idok0e December 30, 2021 19:05 Inactive
@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-idok0e December 31, 2021 06:45 Inactive
@yathomasi yathomasi marked this pull request as ready for review December 31, 2021 06:50
@yathomasi yathomasi requested a review from julieg18 December 31, 2021 06:51
@shcheklein shcheklein added A: website Area: website website: eng-doc DEPRECATED JS engine for /doc 👌 restyled labels Dec 31, 2021
@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-idok0e January 3, 2022 08:53 Inactive
@yathomasi
Copy link
Contributor Author

@iterative/docs
Some assumptions here are:

  1. arguments are always enclosed in square brackets eg: [-h]
  2. arguments that need to be linked start with - or --. eg: [-h] [--version]
  3. Option definition on the list starts with the respective inline code block which also starts with - or --
    Eg:
  • -h

These are the assumptions taken and these need to be converted into guidelines/rules.
For my quick overview look, I could not see edge cases for now. Please, mention them if there could be some.

@julieg18 julieg18 temporarily deployed to dvc-org-interactive-syn-zo8hwd January 3, 2022 17:48 Inactive
@casperdcl
Copy link
Contributor

These are the assumptions taken and these need to be converted into guidelines/rules.

It's just the output from dvc ... --help as generated by argparse so I don't think any manual rules are needed.

Copy link
Contributor

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good

gatsby-config.js Outdated Show resolved Hide resolved
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I did notice some minor things, but I think we could still merge this and fix them on a later iteration.

config/prismjs/usage.js Outdated Show resolved Hide resolved
plugins/gatsby-remark-dvc-linker/commandLinker.js Outdated Show resolved Hide resolved
@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-s47nhu January 8, 2022 06:22 Inactive
@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-5cbgfp January 8, 2022 06:25 Inactive
@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-5cbgfp January 10, 2022 05:34 Inactive
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome! Reading through all of the comments, there is still a couple things to do but we could create a issue listing the rest of the tasks to do later. If everyone else agrees, I think we could merge this!

Copy link
Contributor

@rogermparent rogermparent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely looks good enough to me to be merged as a first iteration!

Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Good stuff with the prism hook - less code == better sleep :)

Two small things. First, could we please move icon a bit down to align it better with the bullets?

Screen Shot 2022-01-10 at 4 18 39 PM

And please still review and resolve the existing comments (it's totally fine if you think that it's not the right time to address, or you want to keep certain things the way it is, but it's a good practice to react and say that we are moving forward).

@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-5cbgfp January 11, 2022 07:20 Inactive
@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-5cbgfp January 11, 2022 08:42 Inactive
@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-5cbgfp January 11, 2022 09:16 Inactive
@yathomasi
Copy link
Contributor Author

We had headings icons aligned similarly, so I had left it as it was.
I have just made the change and this looks nice with bullet points.
Screen Shot 2022-01-11 at 15 00 11
If we want more aesthetic changes, we can continue on another iteration.

Similarly, looks like I had missed another conversation. I have gone through and updated them with the necessary changes.

Here is the listing we would need discussion or move to another iteration:

  • plain mentions of options inside a cmd ref. could be auto-linked to that anchor e.g. mention of --no-commit, which would be linked to #--no-commit
  • making args-linker more strict to the Options section
  • making aesthetic/UI changes for the anchor tags on synopsis or anchor icon on list options.

I think these are all, please mention if I have missed any.

@yathomasi yathomasi temporarily deployed to dvc-org-interactive-syn-j8lbc5 January 11, 2022 09:44 Inactive
@yathomasi yathomasi requested a review from shcheklein January 11, 2022 10:22
@shcheklein
Copy link
Member

Thanks @yathomasi !

@shcheklein shcheklein merged commit 15aac29 into master Jan 11, 2022
@shcheklein shcheklein deleted the interactive-synopsis branch January 11, 2022 19:25
iesahin pushed a commit that referenced this pull request Apr 11, 2022
* added html to react

* utility html to react parser

* using code component and minor changes

* added args linker plugin to add ids to options

* removed hover css

* fixed selection logic

* removed html to react dependency

* added anchor icon in front of list items

* update to fix only use args from square brackets

* show anchor icon on list hover

* added outline

* linkified whole arguments with parameters

* allowed two level of square brackets nestings

* made link icon visible on focus

* updated command linker to add hash links for args

* fix codeclimate

* added id attribute to respective inline code block

* added id to list and paragraph instead

* updated code component to use all props

* added ids for argument to respective code block

* used lodash has property for key check

* replaced custom component with prism hook

* generalized the arg regex

* updated command linker

* reduced line for code climate fix

* used same link icon source for consistency

* added pathname params

* code fixes

* aligned the icons
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: website Area: website website: eng-doc DEPRECATED JS engine for /doc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants