Add two new printer options: -knl and -bnl #463
Conversation
|
Thanks for the patch. It's clear that you spent some effort here, which puts me in a difficult position because this feature was brought up and rejected in #229. In the future, I'd strongly encourage you to raise an issue or talk to upstream developers before writing new features, especially if they mean new APIs or command line flags. Those changes tend to be delicate and require careful consideration, so jumping straight to a patch forces upstream's hand to merge or reject. For now, I'll reopen the issue, as there was another person interested in this feature recently. And as said before, I don't think I'll be merging this PR until there's at least some consensus in #229. Edit: I had originally linked the wrong issue here, apologies. |
|
No worries. I should have done some research before submitting the code. Thanks for re-opening the issue. |
|
Could you please trim down this PR to follow option 2 in #229 (comment)? That is, We also need more test cases - for example, inputs like the following (imagine tabs for indentation): |
|
Sure, it's on my todo-list. I am a bit busy at the moment, but within a week or two I should have time. |
MorganAntonsson
force-pushed
the
knl_bnl_options
branch
from
879b774 to
ab5599f
Compare
|
Hi. I have made the update. Let me know if it's OK. :) |
cmd/shfmt/main.go
Outdated
| @@ -91,6 +92,7 @@ Printer options: | |||
| -ci switch cases will be indented | |||
| -sr redirect operators will be followed by a space | |||
| -kp keep column alignment paddings | |||
| -fn place function braces on a new line | |||
There was a problem hiding this comment.
function opening braces are placed on a separate line
| @@ -962,6 +968,7 @@ func (p *Printer) command(cmd Command, redirs []*Redirect) (startRedirs int) { | |||
| case *Block: | |||
| p.WriteByte('{') | |||
| p.wantSpace = true | |||
| p.wantNewline = p.wantNewline || p.functionNewLine | |||
There was a problem hiding this comment.
Add a comment, like:
// Forbid "foo()\n{ bar; }"
MorganAntonsson
force-pushed
the
knl_bnl_options
branch
from
ab5599f to
7f6eca6
Compare
|
Updated. Sure, feel free to edit the commit message. Thanks for accepting the change! |
Hi,
I have added two new printer options for a less compact style.
-knl(keyword new line) will put keywords like "then" and "do" on a new line.Example:
-bnl(brace new line) will put braces on new lines.Example:
It would be great if you would accept the patch. Let me know if there is anything I need to change.
Regards,
Morgan