Change to code-block:: text from code-block:: bash & update the version of pip#937
Change to code-block:: text from code-block:: bash & update the version of pip#937bhrutledge merged 8 commits intopypa:mainfrom dukecat0:Fixes-#934
code-block:: text from code-block:: bash & update the version of pip#937Conversation
code-block :: text from code-block:: bash & update the version of pipcode-block:: text from code-block:: bash & update the version of pip
|
Hmm... This PR make me think that we should not checkin the |
Thanks for your suggestion and I will open another PR for this. |
bhrutledge
left a comment
There was a problem hiding this comment.
Thanks for the comprehensive update! Beyond the question of text vs other lexers, just a couple some small tweaks to installing-using-pip-and-virtual-environments.rst for consistency.
| .. code-block:: bash | ||
|
|
||
| pip 21.1.3 from $HOME/.local/lib/python3.9/site-packages (python 3.9) |
There was a problem hiding this comment.
This isn't a bash command, but rather its output, so I think it should be text:
| .. code-block:: bash | |
| pip 21.1.3 from $HOME/.local/lib/python3.9/site-packages (python 3.9) | |
| .. code-block:: text | |
| pip 21.1.3 from $HOME/.local/lib/python3.9/site-packages (python 3.9) |
There was a problem hiding this comment.
Also, I think these blocks under "Activating Virtual Environments" should similarly separate the command from its output:
.. tab:: Unix/macOS
.. code-block:: bash
which python
.../env/bin/python
.. tab:: Windows
.. code-block:: text
where python
...\env\Scripts\python.exeThere was a problem hiding this comment.
I'm pretty sure it's console rather than text.
There was a problem hiding this comment.
"Text" removes all semantic meaning from the highlighting, promising you will never do better than text, and making it programmatically indistinguishable from a text output. I'd think bat or powershell would be better, as that's actually the language this is in, even if there's no current visual difference. console is supposed to be somewhat agnostic, even if bash-like - was it rendering incorrectly? Also, if they are struggling with output vs. input, what about:
.. code-block:: bat
where python
.. code-block:: text
...\env\Scripts\python.exe? Does that break it up too much? I used to have a "input" and "output" type (gitbook plugin, IIRC) that would handle this nicely. As long as we are consistent, it's easy to refactor in the future - going to "text" makes it harder, because there may really be some "text" blocks that are not Windows shell inputs.
There was a problem hiding this comment.
Those are good points re: semantics and refactoring, @henryiii. Since I was the one who originally suggested text, I just pushed the change to bat for Windows inputs (ee40d83), plus a couple remaining bash for Unix inputs (4e01f46).
I don't think console is correct for outputs, because according to the docs, it's a "lexer for Bash shell sessions, i.e. command lines, including a prompt, interspersed with output". text is widely-used throughout this project for outputs.
There was a problem hiding this comment.
@bhrutledge I think I've seen console perform well with windows-based prompts. My understanding that it only highlights the lines starting with some sort of a prompt differently from those that don't.
But yes, looking at https://github.com/pygments/pygments/blob/1ea5fc3/pygments/lexers/shell.py#L226 and https://github.com/pygments/pygments/blob/1ea5fc3/pygments/lexers/shell.py#L556, it seems like doscon is a windows counterpart of the console lexer.
@henryiii I do think that having clear input (with prompt) + output coupled would be beneficial
Co-authored-by: Brian Rutledge <brian@bhrutledge.com>
bhrutledge
left a comment
There was a problem hiding this comment.
Thanks for the changes. I pushed a couple more tweaks to the installing-using-pip doc.
Since this would be my first merge as a maintainer, I'll leave this open for another day to allow for more comments.
|
|
||
| It should be in the ``env`` directory: | ||
|
|
||
| .. tab:: Unix/macOS | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| .../env/bin/python | ||
|
|
||
| .. tab:: Windows | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| ...\env\Scripts\python.exe |
There was a problem hiding this comment.
I thought having more distinction between the command and its output would read better.
| Afterwards, you should have the latest version of pip installed in your | ||
| user site: | ||
|
|
||
| python3 -m pip --version | ||
| pip 9.0.1 from $HOME/.local/lib/python3.6/site-packages (python 3.6) | ||
| .. code-block:: text | ||
|
|
||
| pip 21.1.3 from $HOME/.local/lib/python3.9/site-packages (python 3.9) |
There was a problem hiding this comment.
In retrospect, the changes to this section probably should be in a separate PR, but I took the opportunity to (hopefully) add some additional clarity.
There was a problem hiding this comment.
Yep, also, the console session should be coupled with the output after the prompt input.
There was a problem hiding this comment.
coupled with the output after the prompt input
If you mean there shouldn't be any explanatory text between the input and the output code-block, I disagree. That's how it was in earlier commits, and I thought it was a bit confusing. Also, there are already places in this doc (and I assume others) where they are separated, e.g.:
There was a problem hiding this comment.
It's confusing when the input line isn't marked with a prompt. If it is, that should be rendered better.
|
Thanks for @henryiii's suggestion and @bhrutledge's commits! :) |
|
@bhrutledge Do you think we should add copy button for folks so they can copy the command directly? It's very easy to do this( just add a sphinx extension). Btw, this could be added in another PR. |
| .. code-block:: bat | ||
|
|
||
| py -m pip install twine |
There was a problem hiding this comment.
For win console, it's probably just
| .. code-block:: bat | |
| py -m pip install twine | |
| .. code-block:: doscon | |
| > py -m pip install twine |
There was a problem hiding this comment.
Or maybe
| .. code-block:: bat | |
| py -m pip install twine | |
| .. code-block:: ps1con | |
| $ py -m pip install twine |
There was a problem hiding this comment.
D'oh. I missed those. I agree those seem appropriate, if there's a parallel change of all of the bash inputs, i.e.:
.. code-block:: console
$ twine upload dist/*
I think that's beyond the scope of this PR. Also, as noted in an earlier discussion, I'm not sure it's a better experience, because it adds a little more work to copying a command.
bhrutledge
left a comment
There was a problem hiding this comment.
Thanks for the additional comments. I'm going to merge this as-is, because I think it's a nice improvement. Folks can follow up with additional issues/PRs if they like.
Do you think we should add copy button for folks so they can copy the command directly?
@meowmeowmeowcat That sounds handy. I'd probably start with a PR that just adds the button, but not the prompt characters and the configuration to remove it.
| .. code-block:: bat | ||
|
|
||
| py -m pip install twine |
There was a problem hiding this comment.
D'oh. I missed those. I agree those seem appropriate, if there's a parallel change of all of the bash inputs, i.e.:
.. code-block:: console
$ twine upload dist/*
I think that's beyond the scope of this PR. Also, as noted in an earlier discussion, I'm not sure it's a better experience, because it adds a little more work to copying a command.
| Afterwards, you should have the latest version of pip installed in your | ||
| user site: | ||
|
|
||
| python3 -m pip --version | ||
| pip 9.0.1 from $HOME/.local/lib/python3.6/site-packages (python 3.6) | ||
| .. code-block:: text | ||
|
|
||
| pip 21.1.3 from $HOME/.local/lib/python3.9/site-packages (python 3.9) |
There was a problem hiding this comment.
coupled with the output after the prompt input
If you mean there shouldn't be any explanatory text between the input and the output code-block, I disagree. That's how it was in earlier commits, and I thought it was a bit confusing. Also, there are already places in this doc (and I assume others) where they are separated, e.g.:
|
re:copybutton — it'd be nice to integrate https://github.com/executablebooks/sphinx-copybutton + https://github.com/python/python-docs-theme/blob/master/python_docs_theme/static/copybutton.js |
Thanks for this! |
@webknjaz Is this necessary for the docs? I think most of the code blocks in the docs are shell commands. |
|
Dunno, I think it'd be nice to have the ability to easily copy those commands w/o the prompt. |
|
I agree. I will test sphinx-copybutton and see if it can integrate the function of the copybutton in python docs . |
Uh oh!
There was an error while loading. Please reload this page.