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

Option -h doesn't respect comment newlines. #358

Open
Nifty255 opened this issue Jul 28, 2021 · 4 comments · May be fixed by #459
Open

Option -h doesn't respect comment newlines. #358

Nifty255 opened this issue Jul 28, 2021 · 4 comments · May be fixed by #459
Labels

Comments

@Nifty255
Copy link

Nifty255 commented Jul 28, 2021

Given the following target (imported into main magefile):

// Start creates and starts a new MongoDB container using the specified params.
// With the exception of name, all params can use defaults by using "-" for each.
// Params:
// - name         required
// - version      optional  default 4.4
// - network      optional  default no network
// - hostPort     optional  default 27017
// - replicatSet  optional  default no rs
// - templateFile optional  default no template restored
func Start(name, version, network, hostPort, replicaSet, templateFile string) error {

The following is the output of -h:

$ mage -h mongo:start
Start creates and starts a new MongoDB container using the specified params. With the exception of name, all params can use defaults by using "-" for each. Params: - name         required - version      optional  default 4.4 - network      optional  default no network - hostPort     optional  default 27017 - replicaSet   optional  default no rs - templateFile optional  default no template restored

Usage:

        mage mongo:start <name> <version> <network> <hostPort> <replicaSet> <templateFile>

Expected behavior would be to print the comments with their newlines as written in the comments describing the imported target, like so:

$ mage -h mongo:start
Start creates and starts a new MongoDB container using the specified params.
With the exception of name, all params can use defaults by using "-" for each.
Params:
- name         required
- version      optional  default 4.4
- network      optional  default no network
- hostPort     optional  default 27017
- replicaSet   optional  default no rs
- templateFile optional  default no template restored

Usage:

        mage mongo:start <name> <version> <network> <hostPort> <replicaSet> <templateFile>
@natefinch
Copy link
Member

I see two problems here, and one of them is definitely a mage bug. Mage scrapes the go doc for the function. If you run go doc for a function with those comments, it will look the same. Go doc doesn't respect single line returns. When I've had to write docs like that, I've had to use double spaces at the beginning of each newline to tell go doc that this part of the docs are pre-formatted.

So if you do this, go doc will look right:

// Start creates and starts a new MongoDB container using the specified params.
// With the exception of name, all params can use defaults by using "-" for each.
// Params:
//  - name         required
//  - version      optional  default 4.4
//  - network      optional  default no network
//  - hostPort     optional  default 27017
//  - replicatSet  optional  default no rs
//  - templateFile optional  default no template restored
func Start(name, version, network, hostPort, replicaSet, templateFile string) error {

(note there are two spaces before each dash)

However, even doing that does not produce good output with mage.

This will take a little looking into. I think we had stripped out line returns on the assumption that the terminal would wrap it for you.... but that's not the right thing to do for preformatted stuff like this.

If go doc writes the right thing (which it seems to), we might be able to just steal that text directly.

@pablojimpas
Copy link

pablojimpas commented Mar 1, 2023

Wouldn't this be solved by just removing the following call to toOneLine ?

Description: toOneLine(p.Doc),

Is this something else that I'm missing on why that's needed @natefinch? It makes sense to convert comments into one-liners for targets help messages, but not for the package comment.

Edit: I've opened a PR #459 that resolves this issue with the approach described above.

pablojimpas added a commit to pablojimpas/mage that referenced this issue Mar 1, 2023
@pablojimpas pablojimpas linked a pull request Mar 1, 2023 that will close this issue
@natefinch
Copy link
Member

Hmm. I had problems with keeping the original newlines, because the width of your editor window might be wider than your terminal window... And while the editor will scroll off the right side with a horizontal scrollbar, the terminal just wraps it and makes it super ugly.

It's not great. It would definitely be better to have some middle ground that could preserve lone returns without messing up all the formatting from ugly line wraps.

@pablojimpas
Copy link

pablojimpas commented Mar 2, 2023

Formatting in the terminal is always a tricky problem, but I think that whatever go doc does should be good enough for this project. In fact, I think this tool should behave exactly the same, and I've updated my PR to mimic this behaviour using: https://pkg.go.dev/go/doc#Package.Text instead of the raw doc string.

Ugly terminal wraps in widths smaller than 80 characters sucks, but the current behaviour of mixing everything in just one line it's worse, specially with current decade wide & ultra-wide high-res monitors.

pablojimpas added a commit to pablojimpas/mage that referenced this issue May 1, 2023
pablojimpas added a commit to pablojimpas/mage that referenced this issue Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants