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

Add shell annotations on code blocks in English docs. #1801

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

techdragon
Copy link

@techdragon techdragon commented Aug 21, 2023

Changes in this pull request:

  • Markdown code annotations for shell code in multiple pages of the English documentation.

Added annotations for a lot of markdown code blocks that were missing the shell annotation to highlight them correctly.
Copy link
Contributor

@elena elena left a comment

Choose a reason for hiding this comment

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

Straightforward, nice solutiont to the dollar signs copy/paste problem

C:\Users\Name\djangogirls> myvenv\Scripts\activate
```

> __NOTE:__ On Windows 10 you might get an error in the Windows PowerShell that says `execution of scripts is disabled on this system`. In this case, open another Windows PowerShell with the "Run as Administrator" option. Then try typing the following command before starting your virtual environment:
>
>{% filename %}command-line{% endfilename %}
>```
>```powershell
Copy link
Contributor

Choose a reason for hiding this comment

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

🌟

Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I like the idea but if it starts with $ (or #) and then a shell command it's actually console.

The problem is that we also have many examples where it writes the full prompt instead of $. That's usually not a bad thing because the path does provide additional context that's useful, but it doesn't work well with syntax highlighting. I'm not sure what's best there.

@@ -24,7 +24,7 @@ Now you should see an interface with a sidebar, buttons at the left.
Click "Terminal" button to open terminal window with prompt like this:

{% filename %}Terminal{% endfilename %}
```
```shell
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't shell, but console

@@ -53,7 +53,7 @@ Now you should see an interface with a sidebar, a big main window with some
text, and a small window at the bottom that looks something like this:

{% filename %}bash{% endfilename %}
```
```shell
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is more like console, but that doesn't recognize a full prompt (like a full PS1 variable: https://docs.pulpproject.org/pulpcore/configuration/settings.html#pulp-settings).

@@ -69,7 +69,7 @@ bigger.
4. Click on the Tools dropdown list (at the bottom left side of the window), then on Terminal button to open terminal tab with a prompt like this:

{% filename %}Terminal{% endfilename %}
```
```shell
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

@@ -29,7 +29,7 @@ All you need to do is find a directory in which you want to create the `virtuale
For this tutorial we will be using a new directory `djangogirls` from your home directory:

{% filename %}command-line{% endfilename %}
```
```shell
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is very common in this file, so I'm not commenting it on every line.

Suggested change
```shell
```console

@techdragon
Copy link
Author

techdragon commented Aug 22, 2023

I used shell as the docs appear to be using HighlightJS, and the documentation says that shelland console are both aliases for the same thing. https://highlightjs.readthedocs.io/en/latest/supported-languages.html

Having never used honkit I'm not completely sure if there's any preprocessing that would get in the way and complicate this situation though... but I based the PR on the HighlightJS docs since the honkit code for syntax highlighting (https://github.com/honkit/honkit/tree/d058ddfcd0837af110c9f2d49f612bc0b75390b4/packages/%40honkit/honkit-plugin-highlight) looks like its just a wrapper around HighlightJS.

Edit:
Would you still like the shell swapped for console even though they should be functional equivalent aliases @ekohl ?

@shweta-borganve
Copy link

Hello, could you enable me for write access to the repo? I would like to commit changes by resolving conflicts.
Thank you

@das-g
Copy link
Member

das-g commented Oct 30, 2023

Hello, could you enable me for write access to the repo? I would like to commit changes by resolving conflicts. Thank you

Which repo, @shweta-borganve? For contributions to https://github.com/DjangoGirls/tutorial please "fork" it on GitHub, make your changes in your "fork" or push them there, then file a pull request. For https://github.com/techdragon/tutorial, @techdragon would have to decide whether to give you push access, but also there you could work with pull requests.

Is this related to this PR here (#1801 «Add shell annotations on code blocks in English docs.»)? If needed I can give some pointers on how to get its commit into your own fork of https://github.com/DjangoGirls/tutorial, so that you can build upon it.

@shweta-borganve
Copy link

@das-g Thank you very much for details, I will proceed accordingly and contact if needed.

@fltfx
Copy link
Member

fltfx commented Oct 13, 2024

@techdragon @ekohl Any updates on this or can we close?

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.

6 participants