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

docs: wrong use of diff highlighting + indentation problem #1761

Closed
bobertlo opened this issue Sep 5, 2020 · 9 comments · Fixed by #2359
Closed

docs: wrong use of diff highlighting + indentation problem #1761

bobertlo opened this issue Sep 5, 2020 · 9 comments · Fixed by #2359
Labels
A: docs Area: user documentation (gatsby-theme-iterative) 🐛 type: bug Something isn't working.

Comments

@bobertlo
Copy link
Contributor

bobertlo commented Sep 5, 2020

UPDATE: Jump to #1761 (comment)


I have opened an issue #1760 to fix the only important example, but I wonder if the rest of the 'diff' formatted code blocks might benefit from this? This PR is the only one that really sticks out but it is also slightly noticeably elsewhere. If there was positive feedback on this I would be happy to find other cases of this.

@jorgeorpinel
Copy link
Contributor

I think we should indeed review all diff code blocks for consistency (some have more or less spaces after + for example) but Idk if we want to fix the indentation manually. Maybe better to report in https://github.com/PrismJS/prism/issues first?

@jorgeorpinel jorgeorpinel changed the title docs: diff offsets are rendered non-intuitively docs: diff highlight does not offsets some lines (indentation) Sep 5, 2020
@jorgeorpinel jorgeorpinel added the dependencies Pull requests that update a dependency file (automatic) label Sep 5, 2020
@jorgeorpinel jorgeorpinel added website: eng-blog DEPRECATED JS engine for /blog status: research Writing concrete steps for the issue labels Sep 15, 2020
@jorgeorpinel

This comment has been minimized.

@jorgeorpinel
Copy link
Contributor

UPDATE: So, as pointed out by @efiop in https://github.com/iterative/dvc.org/pull/1760/files#r487469021, our current diff code blocks aren't actually diffs or patches. They intend to use whatever format git diff outputs. So we should probably not use the diff language in markdown.

So we can use dvc or no language. AND we should then probably align the lines manually, with a single space for lines that don't start with +.

@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) and removed dependencies Pull requests that update a dependency file (automatic) status: research Writing concrete steps for the issue website: eng-blog DEPRECATED JS engine for /blog labels Sep 15, 2020
@jorgeorpinel jorgeorpinel changed the title docs: diff highlight does not offsets some lines (indentation) docs: we're using diff highlighting in MD wrong Sep 15, 2020
@rogermparent
Copy link
Contributor

I checked this out a little on my own when it popped up, and if I were to guess it seems to be an interaction between the indented Markdown list and however the code block is passed to Prism.

Here's all the existing diff blocks in the repo:

content/docs/start/data-access.md
78   ```diff

content/docs/start/data-pipelines.md
95     ```diff
164  ```diff

content/docs/command-reference/import-url.md
174  ```diff

content/docs/start/experiments.md
168  ```diff

The table that was pointed out in data-pipelines is the only one that has the issue, and the others I've looked at appear to be aligned correctly when compared to the source.

@jorgeorpinel
Copy link
Contributor

Thanks for the list, we should look at all of those. And as an update to my previous message:

our current diff code blocks aren't actually diffs or patches

Actually we (and git diff) use patch format, but Prism doesn't seem to support that. Diff format is something else (what Git merge conflicts use).

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Sep 15, 2020

Another UPDATE:

AND we should then probably align the lines manually, with a single space

Unfortunately, using a single space like git diff would output doesn't seem to be displayed correctly by our engine for code blocks that are indented themselves . See #1760 (review)
Like Roger mentioned:

seems to be an interaction between the indented Markdown list and however the code block is passed to Prism

@jorgeorpinel jorgeorpinel changed the title docs: we're using diff highlighting in MD wrong docs: we're using diff highlighting in MD wrong + block indentation bug Sep 15, 2020
@jorgeorpinel jorgeorpinel added the 🐛 type: bug Something isn't working. label Sep 15, 2020
@jorgeorpinel jorgeorpinel changed the title docs: we're using diff highlighting in MD wrong + block indentation bug docs: wrong use of diff blocs + indentation problem Sep 15, 2020
@jorgeorpinel jorgeorpinel added hacktoberfest help wanted Contributors especially welcome labels Sep 15, 2020
@bobertlo
Copy link
Contributor Author

What if we changed the way we think about these listing? The core problem seems to be broken formatting in dirtree diff output, which is sort of an artificial concept.

Maybe we could look at this less as a 'look under the hood' and more of a 'verify your progress' and provide (sometimes abridged) outputs of reproducible commands:

  1. If a user is to change the params file, provide a command git diff and mock the actual output expected.

  2. If changes to multiple files are made, provide the appropriate git status or dvc status with (possibly abridged) expected outputs.

I think this would solve the formatting problem because it doesn't really matter in an actual diff context and as a bonus the sections would be more clear and useful (IMO). Saying 'what happens under the hood' sort of implies internal processes of DVC whereas looking at it from this angle implies more looking at and verifying the output of the processes.

Maybe this is a bit radical, and probably would be a separate issue, but it would close this issue I think.

@jorgeorpinel
Copy link
Contributor

Hmmm sounds like much more effort than addressing this very specific formatting issue. And

  1. If changes to multiple files are made, provide the appropriate git status

The output of git status will probably have the same problem. Unless I didn't get you right, of course.

Thanks for the ideas though.

@jorgeorpinel
Copy link
Contributor

UPDATE: The YAML part is also mentioned in #1509. The diff part should be fixed by #2359 (which will close this issue).

@jorgeorpinel jorgeorpinel removed hacktoberfest help wanted Contributors especially welcome labels May 9, 2021
@jorgeorpinel jorgeorpinel changed the title docs: wrong use of diff blocs + indentation problem docs: wrong use of diff highlighting + indentation problem May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative) 🐛 type: bug Something isn't working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants