Skip to content

Conversation

@hoechenberger
Copy link
Member

  • The command line meant to run the installer on Linux now comes with a copy button
  • Switch from .. code-block:: console to ... bash everywhere, as Pygments, which is reliable for the syntax highlighting, doesn't know any console
  • Now always show the copy button, not only on hovering above the code blocks. I believe this makes this functionality easier to discover.
  • Add manual installation instructions for Apple Silicon.

@larsoner
Copy link
Member

larsoner commented May 2, 2022

Switch from .. code-block:: console to ... bash everywhere, as Pygments, which is reliable for the syntax highlighting, doesn't know any console

Did you consider bash session instead? https://pygments.org/languages/ It seems like a more appropriate name for our "BASH prompt" type of examples, but I haven't tested. I can test locally if needed...

@hoechenberger
Copy link
Member Author

@hoechenberger hoechenberger marked this pull request as ready for review May 2, 2022 15:17
@hoechenberger
Copy link
Member Author

Did you consider bash session instead?

No, is there a difference?

@hoechenberger
Copy link
Member Author

Did you consider bash session instead?

No, is there a difference?

Ah it seems to add a prompt.

I'd rather we didn't do that tbh, it's confusing because the "example prompt" never matches what the users actually see on their own system…

@larsoner
Copy link
Member

larsoner commented May 2, 2022

I'd rather we didn't do that tbh, it's confusing because the "example prompt" never matches what the users actually see on their own system…

I think having the $ is informative/helpful

@hoechenberger
Copy link
Member Author

I think having the $ is informative/helpful

I'd argue that's only the case for those "who know"; and those who know already know and don't need this.

For all others, it's confusing – especially on Windows.

@larsoner
Copy link
Member

larsoner commented May 2, 2022

I'd argue that's only the case for those "who know"; and those who know already know and don't need this.

I disagree on this, I think it helps people who don't know. They see >>> locally they're more likely to know they're "in the wrong place" if we have $ in our instructions.

For all others, it's confusing – especially on Windows.

Again I disagree here -- on Windows if you don't see $ but instead the CMD prompt, it's entirely possible that whatever command we are telling you to do will not work because it's bash-specific. I think having the $ makes explicit that we are currently making guarantees for BASH-like prompts only. The windows users who don't see this should be a bit worried that they might not be able to do some of the things we suggest!

There might be places where we are command-prompt-agnostic in the sense that commands happen to work in CMD, PowerShell, bash, zsh, tcsh, etc. but I think our docs were not written with this is mind for the most part.

@hoechenberger
Copy link
Member Author

Alright, but we shouldn't have this in the installation instructions, where we lump together the prompts for several operating systems that DO work in all supported shells; e.g. here, the command to create a conda env on macOS, Windows, and Linux:

https://output.circle-artifacts.com/output/job/779f2502-8921-410e-b161-122126b094b5/artifacts/0/dev/install/manual_install.html

@larsoner
Copy link
Member

larsoner commented May 2, 2022

Alright, but we shouldn't have this in the installation instructions, where we lump together the prompts for several operating systems that DO work in all supported shells; e.g. here, the command to create a conda env on macOS, Windows, and Linux:

I can live with no $ there. For consistency maybe we should use the same phrasing "the terminal or an Anaconda Prompt" as in https://docs.conda.io/projects/conda/en/latest/user-guide/tasks/manage-environments.html

@hoechenberger
Copy link
Member Author

Sounds good to me! Do you want to do these changes?

@larsoner
Copy link
Member

larsoner commented May 2, 2022

No, feel free since you're working on this stuff

@hoechenberger
Copy link
Member Author

Ouch, turns out Pygments does support console – it's an alias for shell-session.

The reason why this didn't render correctly in some places was …

… because it expects a leading $ (or any other prompt – some auto detection seems to be going on there)

So I'll revert 283a181 and try to fix the occasions where things don't seem to be rendered correctly.

@hoechenberger
Copy link
Member Author

Ok … let's hope this comes back looking nicely.

Turns out you're getting it your way, @larsoner ;)

@hoechenberger
Copy link
Member Author

@larsoner Should be good to merge once CI is done!!

@hoechenberger
Copy link
Member Author

@hoechenberger hoechenberger changed the title Updates to the installation instructions & small tweaks to other parts of the documentation MRG: Updates to the installation instructions & small tweaks to other parts of the documentation May 2, 2022
@larsoner larsoner merged commit cf820df into mne-tools:main May 3, 2022
@larsoner
Copy link
Member

larsoner commented May 3, 2022

Thanks @hoechenberger !

@hoechenberger hoechenberger deleted the install-docs-new branch May 3, 2022 12:24
@hoechenberger
Copy link
Member Author

@larsoner Do you think we can backport this?

@larsoner
Copy link
Member

larsoner commented May 3, 2022

Feel free, especially since it looks like #10580 was backported and this PR has fixes for some of the code blocks changed there

@hoechenberger hoechenberger added the backport-candidate on-merge: backport to maint/1.11 label May 3, 2022
hoechenberger added a commit that referenced this pull request May 3, 2022
@hoechenberger hoechenberger added backported and removed backport-candidate on-merge: backport to maint/1.11 labels May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants