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

Shell code snippets include command prompt #12739

Closed
1 of 2 tasks
sftim opened this issue Feb 20, 2019 · 12 comments
Closed
1 of 2 tasks

Shell code snippets include command prompt #12739

sftim opened this issue Feb 20, 2019 · 12 comments

Comments

@sftim
Copy link
Contributor

sftim commented Feb 20, 2019

This is a...

  • Feature Request
  • Bug Report

Problem:
As per the current style guide, snippets of code shouldn't include the command prompt. On several pages there are code snippets that do include a prompt.

In some places this is mixed in with output, eg (Markdown):

```shell
$ printf "%s\n" "Foo bar"
Foo bar
```

Proposed Solution:
So, the style guide is clear. I'll amend the issue description.
I think a mixture of approaches.

We want to be able to show what to type mixed with sample output. So the style guide should cover that case and explain what to do.
The site generator might need changes to handle highlighting those blocks, treating the initial $ as a hint that the rest of the line is code, and lines not starting with $ as unformatted command output.

Once those changes are made we can go in and PR fixes to use the new style consistently.
Follow style guide: separate commands from output.

@DanyC97
Copy link
Contributor

DanyC97 commented Feb 20, 2019

@sftim can you provide an example of type mixed with sample output ?

@sftim
Copy link
Contributor Author

sftim commented Feb 20, 2019

Ah, sorry. I edited that out whilst drafting @DanyC97

I've updated the issue description.

@sftim
Copy link
Contributor Author

sftim commented Feb 20, 2019

The pages that I think need fixing (en-US locale) are:

@iamneha
Copy link
Contributor

iamneha commented Feb 21, 2019

I would like to submit PR for this issue

/assign @iamneha

@k8s-ci-robot
Copy link
Contributor

@iamneha: GitHub didn't allow me to assign the following users: iamneha.

Note that only kubernetes members and repo collaborators can be assigned and that issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

I would like to submit PR for this issue

/assign @iamneha

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@DanyC97
Copy link
Contributor

DanyC97 commented Feb 21, 2019

@iamneha before fire any PRs, could you please detail how you planning to tackle, maybe a PR on the style guide docs and a PR example will be okay.

Doing so then @sftim, myself or anyone else can pick it up and make sure that we are coordinating on the effort

@iamneha
Copy link
Contributor

iamneha commented Feb 22, 2019

@sftim @DanyC97 I created a PR, please check here #12779
This is a reference PR so that it would be easy to review

If this looks fine I'll add commit for rest of the files as well

@zacharysarah
Copy link
Contributor

@sftim 👋

We want to be able to show what to type mixed with sample output. So the style guide should cover that case and explain what to do.

Do we? Right now, the style guide specifies to separate commands from output. Or do you mean something different?

The site generator might need changes to handle highlighting those blocks, treating the initial $ as a hint that the rest of the line is code, and lines not starting with $ as unformatted command output.

Again, the style guide is clear about separating commands from output.

That said, there is a possibility for improvement where we offer examples in multiple languages: better implementation of shortcodes for tabs. I haven't seen any discussion of them so far, so wanted to raise visibility as a possible path for improvement.

@sftim
Copy link
Contributor Author

sftim commented Feb 26, 2019

Separate commands from output makes sense to me. I missed that when I was reading through before.

So, the style guide is clear. I'll amend the issue description.

@iamneha
Copy link
Contributor

iamneha commented Feb 28, 2019

hi @sftim as you mentioned separate commands from output will work and you updated the description as well.
I want to open this issue again #12779 and I had already done these changes in my PR.
It would be very helpful if you look into it.

@zacharysarah @Rajakavitha1 can you open this #12779 issue again

@iamneha
Copy link
Contributor

iamneha commented Feb 28, 2019

@DanyC97
Copy link
Contributor

DanyC97 commented Feb 28, 2019

@iamneha you might have seen that i went ahead and updated all the pages to remove $

Once that is done i can either update the above PRs or vice-versa (rebase the PRs ).

regarding Separate commands from output i haven't touched it yet, i can do in the above PRs or 1 big one, i don't mind really.

At this point in time i think i need to rethink the strategy on how to fix everything i had in mind because the reality is:

a) there are fewer people with review/ approve permission who are available / can stay on top of all this and pair with me so we can deliver faster this changes
b) because of that it might take a very long time and that is not good either, even for cosmetic with no huge UX impact

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

No branches or pull requests

5 participants