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

Update the linebreak symbol #812

Closed
wants to merge 1 commit into from
Closed

Update the linebreak symbol #812

wants to merge 1 commit into from

Conversation

leehanchung
Copy link

The original doc comment of this is confusing for Python users:

$ mkdir data
$ dvc get https://github.com/iterative/dataset-registry \ 
get-started/data.xml -o data/data.xml

So changed this into one single command line for clarity after conversation with Ivan on Discord.

$ mkdir data
$ dvc get https://github.com/iterative/dataset-registry get-started/data.xml -o data/data.xml

IMPORTANT NOTES (please read, then delete):

  • Have you followed the guidelines in
    Contributing Documentation?

  • Please use the title to provide a clear one-line present-tense summary of the
    changes introduced in the PR. For example: "Introduce the first version of the
    collection editor.".

  • Please make sure to mention "Fix #bugnum" (if applicable) in the description
    of the PR. This enables GitHub to link the PR to the corresponding bug.

The original doc comment of this is confusing for Python users:
```dvc
$ mkdir data
$ dvc get https://github.com/iterative/dataset-registry \ 
get-started/data.xml -o data/data.xml
```
So changed this into one single command line for clarity. 
```dvc
$ mkdir data
$ dvc get https://github.com/iterative/dataset-registry get-started/data.xml -o data/data.xml
```
@jorgeorpinel
Copy link
Contributor

Thanks @leehanchung! Are you VitaminC on Discord? Haha

The reason we have \ after around 70 chars in many of our terminal samples is for better readability though. Otherwise the code blocks would scroll horizontally.

Fortunately we have #759 open that would solve this possible confusion for Windows users. Or if you have any other suggestions, feel free to reach out on Discord or GitHub issues.

@shcheklein
Copy link
Member

@jorgeorpinel can we do this split before -o to reduce chances for mistake? It's not even about Windows users vs Linux - it's also about Python habits it looks like :)

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Nov 22, 2019

That would make the code not fit in the block, which would scroll, @shcheklein:

image

@shcheklein
Copy link
Member

:( yeah, that URL is just too large

not sure though that Windows tabs is the solution for this problem

may be do dvc get -o data/data.xml URL PATH?

@jorgeorpinel
Copy link
Contributor

That still needs \. Probably 2:

$ dvc get -o data/data.xml \
          https://github.com/iterative/dataset-registry \ 
          get-started/data.xml

And we have a bunch of \ break lines all around the docs.

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Nov 23, 2019

not sure though that Windows tabs is the solution for this problem

That should definitely solve this, as \ is not supported in cmd; So for Windows cmd code samples we would have to use long lines and a special code block that wraps lines, I guess. Or a horizontal scroll bar that is always shown so people don't miss the hidden part of the lines.

@leehanchung
Copy link
Author

How about

dvc get https://github.com/\
iterative/dataset-registry  get-started/data.xml -o data/data.xml

Be mindful when copying the above command. Make sure there's a space between repo-url `https://github.com/iterative/dataset-registry` and `get-started/data.xml`. The usage is `dvc get` <repo-url> <repo-folder> -o <data to be pulled>

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Nov 23, 2019

Interesting. That looks super weird though.

I'm not sure adding the note just for some Windows users is a good idea. We would have to do this on every command sample with \ throughout the docs.

Sorry to sound so negative! I think the best solution is #759, which BTW is still on research phase, so it could well end up being an automatic platform detection mechanism that just adds these notes automatically. I think it's best to move this discussion to that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants