Don't include ">" in bash scripts #460

hbina · 2019-10-07T05:04:59Z

This makes copying inconvenient as > ./x.py build -i --stage 1 src/libstd will actually render your x.py file empty...

spastorino · 2019-10-07T13:01:49Z

We could use a $ sign but please check out through the whole guide because there are tons of "```bash" pieces and we would need to update them all.

hbina · 2019-10-07T16:45:37Z

We could use a $ sign

I don't quite understand. Even if I replace > it will simply copy the entire text and $ is not a valid bash command...

please check out through the whole guide because there are tons of "```bash" pieces and we would need to update them all.

Okay, I am going to apply the changes to all of them now....

spastorino · 2019-10-07T17:23:54Z

I don't quite understand. Even if I replace > it will simply copy the entire text and $ is not a valid bash command...

Yeah, I just meant that would be a bit more obvious that you don't need to copy if you have $ it's more clear that you're on a shell. IMO the best solution is to have the $ sign but once you try to highlight all the text the dollar sign is not highlighted.

hbina · 2019-10-07T17:53:10Z

But the "copy to clipboard* button will copy the entire text, including the $...I guess there are no good solution to this?

shepmaster · 2019-10-07T18:33:41Z

```bash pieces

FWIW, it's generally incorrect to use ```bash for code blocks that represent user interaction, especially multiple interactions or with output. That should be reserved for actual bash shell script. As a concrete example, $ denotes a variable usage in bash, # is a comment, and > is a redirect, yet all of those are used as prompt indicators.

spastorino · 2019-10-07T19:15:34Z

Sorry because I may be having hard time to express my idea :). I feel like in these blocks we should have just the thing that needs to be run, in this case ./x.py build -i --stage 1 src/libstd but I guess we may want to apply some css style to this and maybe prepend a prompt indicator.

Also what @shepmaster says is true. I'm not sure what would be the best solution /cc @mark-i-m

tshepang · 2019-10-07T21:26:57Z

I myself use prompt characters only when including command output (for easy visual separation), otherwise I do not add it for copy-paste convenience.

mark-i-m · 2019-10-10T20:01:35Z

@tshepang That seems like a reasonable rule to me

spastorino · 2019-10-11T14:58:52Z

So in order to make progress with this my suggestion is ...

Let's get rid of all of the prompt symbols in the guide
Open an issue to handle script blocks with css/js styling. I think we should add a prompt but as part of presentation and once you select text that part is not selected.

mark-i-m · 2019-10-11T16:05:24Z

@spastorino I don't know how to do #2 in mdbook, but it sounds good to me.

mark-i-m · 2019-11-14T19:38:45Z

@hbina Thanks for the PR! Are you still interested in working on this? If so, could you remove all the > thoughout the book and rebase on top of master?

hbina · 2019-11-14T20:32:19Z

Uh oh, I guess I better start finish reading the book. Sure, I can work on them. I only know how to do the first one tho, I am not entirely knowledgeable in CSS styling stuff

spastorino · 2019-11-14T20:33:10Z

@hbina going over the first one is good enough for now :)

mark-i-m · 2019-11-14T21:50:17Z

I think we can leave CSS for a follow-up pr. For now, perhaps we can start by just removing the characters?

hbina · 2019-11-14T23:30:10Z

My latest commit removes a bunch of > from code blocks that are meant to be executed. I left, and, sometimes added, $ and / where necessary. Plus, those code are not meant to be copied anyway.

spastorino

I see you did you added some of the $ but seems opposed what we are trying to do.
To be honest unsure how to best fix this without doing the css styling idea.

@mark-i-m do you have any other idea?.

spastorino · 2019-11-15T11:49:49Z

src/profiling/with_perf.md

@@ -260,7 +260,7 @@ usually also want to give `--tree-min-percent` or
 `--tree-max-depth`. The result looks like this:

 ```bash
-> perf focus '{do_mir_borrowck}' --tree-callees --tree-min-percent 3
+$ perf focus '{do_mir_borrowck}' --tree-callees --tree-min-percent 3


spastorino · 2019-11-15T11:49:57Z

src/profiling/with_perf.md

@@ -232,7 +232,7 @@ Perhaps we'd like to know how much time MIR borrowck spends in the
 trait checker. We can ask this using a more complex regex:

 ```bash
-> perf focus '{do_mir_borrowck}..{^rustc::traits}'
+$ perf focus '{do_mir_borrowck}..{^rustc::traits}'


spastorino · 2019-11-15T11:50:08Z

src/profiling/with_perf.md

@@ -191,7 +191,7 @@ function of the MIR borrowck is called `do_mir_borrowck`, so we can do
 this command:

 ```bash
-> perf focus '{do_mir_borrowck}'
+$ perf focus '{do_mir_borrowck}'


I thought the text below this are meant to show an example of what will be displayed? In this case, the "copy to clipboard" button is meaningless regardless. So thats why I thought adding $ is okay to convey what would appear in a console. My original PR was really about "texts that are meant to be executed, should be executable from what is copied to the clipboard".

spastorino · 2019-11-15T11:50:24Z

src/how-to-build-and-run.md

@@ -385,7 +385,7 @@ should see a version number ending in `-dev`, indicating a build from
 your local environment:

 ```bash
-> rustc +stage1 -vV
+$ rustc +stage1 -vV


Hmm I see why you added $ there but I'm not sure what to think about this.
cc @mark-i-m

spastorino · 2019-11-15T11:50:39Z

src/profiling/with_perf.md

@@ -311,7 +311,7 @@ could get our percentages relative to the borrowck itself
 like so:

 ```bash
-> perf focus '{do_mir_borrowck}' --tree-callees --relative --tree-max-depth 1 --tree-min-percent 5
+$ perf focus '{do_mir_borrowck}' --tree-callees --relative --tree-max-depth 1 --tree-min-percent 5


mark-i-m · 2019-11-18T16:52:46Z

@spastorino Personally, I think this is ok. One would not copy/paste the whole block anyway in these cases, and I think having the prompt will improve the syntax highlighting of mdbook...

IMHO, we should just merge...

spastorino · 2019-11-18T17:30:31Z

@hbina I've just approved the changes and I was going to merge them but there are some conflicts now, do you mind rebasing?. Sorry for not merging it before and hitting this new conflicts.

mark-i-m · 2019-11-21T15:58:44Z

Umm... looks like rebase went wrong?

hbina · 2019-11-21T17:37:53Z

Oh...that's really bad...
I did git rebase --onto master patch-1 but I guess that's wrong?

Specifically, `> $1` causes it to write into the file $1 if it exist And `> ./x.py` is particularly bad because it overwrite the script with empty spaces...

hbina · 2019-11-21T18:54:02Z

Should be okay now?

mark-i-m · 2019-11-21T18:54:07Z

Yeah, I'm not really sure why that happens. Occasionally I do that too.

For whatever reason, git checkout your-branch; git rebase master seems to fix it. I think that is equivalent to what you ran but for some reason running it again seems to fix things...

mark-i-m · 2019-11-21T18:54:51Z

Yep, that looks much better :)

mark-i-m

Thanks! This looks good to me. We can do more discussion on zulip or in an issue I think.

ghost · 2020-09-20T19:22:04Z

This code blocks should not be marked as bash, there is nothing bash specific in them. They should be marked as sh.

shepmaster · 2020-09-20T19:26:15Z

They should be marked as sh.

I disagree

JohnTitor previously approved these changes Oct 7, 2019

View reviewed changes

mark-i-m mentioned this pull request Nov 14, 2019

">" in bash scripts #515

Open

hbina dismissed JohnTitor’s stale review via cdf5301 November 14, 2019 23:29

spastorino reviewed Nov 15, 2019

View reviewed changes

spastorino previously approved these changes Nov 18, 2019

View reviewed changes

hbina dismissed spastorino’s stale review via e4c4156 November 21, 2019 13:37

Couple of changes to code so that its safe

d09e812

Specifically, `> $1` causes it to write into the file $1 if it exist And `> ./x.py` is particularly bad because it overwrite the script with empty spaces...

mark-i-m approved these changes Nov 26, 2019

View reviewed changes

mark-i-m merged commit d373bca into rust-lang:master Nov 26, 2019

hbina deleted the patch-1 branch September 20, 2020 05:55

hbina mentioned this pull request Sep 20, 2020

README.md: Remove prompts from code blocks rust-lang/rust#76883

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't include ">" in bash scripts #460

Don't include ">" in bash scripts #460

hbina commented Oct 7, 2019

spastorino commented Oct 7, 2019

hbina commented Oct 7, 2019 •

edited

Loading

spastorino commented Oct 7, 2019 •

edited

Loading

hbina commented Oct 7, 2019

shepmaster commented Oct 7, 2019 •

edited

Loading

spastorino commented Oct 7, 2019

tshepang commented Oct 7, 2019

mark-i-m commented Oct 10, 2019

spastorino commented Oct 11, 2019

mark-i-m commented Oct 11, 2019

mark-i-m commented Nov 14, 2019

hbina commented Nov 14, 2019

spastorino commented Nov 14, 2019

mark-i-m commented Nov 14, 2019

hbina commented Nov 14, 2019 •

edited

Loading

spastorino left a comment

spastorino Nov 15, 2019

spastorino Nov 15, 2019

spastorino Nov 15, 2019

hbina Nov 15, 2019

spastorino Nov 15, 2019

spastorino Nov 15, 2019

mark-i-m commented Nov 18, 2019

spastorino commented Nov 18, 2019

mark-i-m commented Nov 21, 2019

hbina commented Nov 21, 2019

hbina commented Nov 21, 2019

mark-i-m commented Nov 21, 2019

mark-i-m commented Nov 21, 2019

mark-i-m left a comment

ghost commented Sep 20, 2020

shepmaster commented Sep 20, 2020

Don't include ">" in bash scripts #460

Don't include ">" in bash scripts #460

Conversation

hbina commented Oct 7, 2019

spastorino commented Oct 7, 2019

hbina commented Oct 7, 2019 • edited Loading

spastorino commented Oct 7, 2019 • edited Loading

hbina commented Oct 7, 2019

shepmaster commented Oct 7, 2019 • edited Loading

spastorino commented Oct 7, 2019

tshepang commented Oct 7, 2019

mark-i-m commented Oct 10, 2019

spastorino commented Oct 11, 2019

mark-i-m commented Oct 11, 2019

mark-i-m commented Nov 14, 2019

hbina commented Nov 14, 2019

spastorino commented Nov 14, 2019

mark-i-m commented Nov 14, 2019

hbina commented Nov 14, 2019 • edited Loading

spastorino left a comment

Choose a reason for hiding this comment

spastorino Nov 15, 2019

Choose a reason for hiding this comment

spastorino Nov 15, 2019

Choose a reason for hiding this comment

spastorino Nov 15, 2019

Choose a reason for hiding this comment

hbina Nov 15, 2019

Choose a reason for hiding this comment

spastorino Nov 15, 2019

Choose a reason for hiding this comment

spastorino Nov 15, 2019

Choose a reason for hiding this comment

mark-i-m commented Nov 18, 2019

spastorino commented Nov 18, 2019

mark-i-m commented Nov 21, 2019

hbina commented Nov 21, 2019

hbina commented Nov 21, 2019

mark-i-m commented Nov 21, 2019

mark-i-m commented Nov 21, 2019

mark-i-m left a comment

Choose a reason for hiding this comment

ghost commented Sep 20, 2020

shepmaster commented Sep 20, 2020

hbina commented Oct 7, 2019 •

edited

Loading

spastorino commented Oct 7, 2019 •

edited

Loading

shepmaster commented Oct 7, 2019 •

edited

Loading

hbina commented Nov 14, 2019 •

edited

Loading