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

Deleted all $ signs in code blocks in the "Composing multiple nodes in a single process" tutorial #2703

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

Trimple
Copy link
Contributor

@Trimple Trimple commented Jun 9, 2022

$ signs prevent from effective use of "copy to clipboard" button. Previous tutorials don't have them in code blocks

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

So I agree with you that having the $ in here makes it not copy-n-pastable.

But the reason we left it like this on this tutorial is that these are really snippets showing a command and the result, not just a copy-n-paste command. We can definitely make this copyable, but if we are going to do that, we should split the result into a separate code-block, as you don't want to paste those into the terminal.

@Trimple
Copy link
Contributor Author

Trimple commented Jun 9, 2022

True, I think for the command results together with commands it makes sense. But it is not needed in the code blocks with only commands. If it won't affect a document stile too much I can put back $ for every code block with result and leave code blocks with only commands without $ signs.

@Trimple
Copy link
Contributor Author

Trimple commented Jun 9, 2022

We can think about the commands with results as console snapshots. Therefore $ is needed.

@clalancette
Copy link
Contributor

True, I think for the command results together with commands it makes sense. But it is not needed in the code blocks with only commands. If it won't affect a document stile too much I can put back $ for every code block with result and leave code blocks with only commands without $ signs.

The problem is that you then have this inconsistent state within the page where some blocks are copyable and some are not. I'd rather have them all be consistent within the page, one way or the other. Given what most of our other pages do, I think we should probably split them into separate command and result blocks across the board.

@Trimple
Copy link
Contributor Author

Trimple commented Jun 10, 2022

I updated the file and squashed the commits. Can you please take another look?
@clalancette

Deleted all $ signs in the code blocks and separated commands from outputs
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This is fantastic, thanks for this update! Will merge and backport this.

@clalancette clalancette merged commit 1daab08 into ros2:humble Jun 23, 2022
@clalancette
Copy link
Contributor

@Mergifyio backport rolling galactic foxy

mergify bot pushed a commit that referenced this pull request Jun 23, 2022
Deleted all $ signs in the code blocks and separated commands from outputs

(cherry picked from commit 1daab08)
mergify bot pushed a commit that referenced this pull request Jun 23, 2022
Deleted all $ signs in the code blocks and separated commands from outputs

(cherry picked from commit 1daab08)
mergify bot pushed a commit that referenced this pull request Jun 23, 2022
Deleted all $ signs in the code blocks and separated commands from outputs

(cherry picked from commit 1daab08)

# Conflicts:
#	source/Tutorials/Intermediate/Composition.rst
@mergify
Copy link
Contributor

mergify bot commented Jun 23, 2022

clalancette pushed a commit that referenced this pull request Jun 23, 2022
Deleted all $ signs in the code blocks and separated commands from outputs

(cherry picked from commit 1daab08)

Co-authored-by: Andrey Dovnar <andrevdovnar@gmail.com>
clalancette pushed a commit that referenced this pull request Jun 23, 2022
Deleted all $ signs in the code blocks and separated commands from outputs

(cherry picked from commit 1daab08)

Co-authored-by: Andrey Dovnar <andrevdovnar@gmail.com>
clalancette pushed a commit that referenced this pull request Jun 23, 2022
Deleted all $ signs in the code blocks and separated commands from outputs

(cherry picked from commit 1daab08)

# Conflicts:
#	source/Tutorials/Intermediate/Composition.rst
clalancette pushed a commit that referenced this pull request Jun 23, 2022
Deleted all $ signs in the code blocks and separated commands from outputs

(cherry picked from commit 1daab08)

# Conflicts:
#	source/Tutorials/Intermediate/Composition.rst

Co-authored-by: Andrey Dovnar <andrevdovnar@gmail.com>
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.

None yet

2 participants