You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The tool is removing wraps that I've added by hand. I think this is going way too far.
Radu raised the issue too in cockroachdb/cockroach#8823 (comment) but a quick discussion concluded that "less options is better". I'd like to revisit that. If I chose to wrap, let it be.
To lawyer up, this rule is not in the style guide. The style guide has vague wording saying to assume code is gonna be read on 100 cols terminals and to wrap at 100, but I'm pretty sure the intent was to set an upper bound.
Before:
func createTestClientForUser(
t *testing.T,
stopper *stop.Stopper,
addr, user string,
dbCtx client.DBContext,
) *client.DB {
After:
func createTestClientForUser(
t *testing.T, stopper *stop.Stopper, addr, user string, dbCtx
) *client.DB {
I don't see a problem if the tool allows this. 100 is indeed an upper bound, and there is no suggestion that you should always try to fit stuff on one line.
I believe this will require a bit more logic to detect if it is correctly formatted (because if it isn't, crlfmt will have to reformat it), so if we implement this I think it would be a good time to add some tests. The test cases could be stored in file pairs (input and expected output).
The tool is removing wraps that I've added by hand. I think this is going way too far.
Radu raised the issue too in cockroachdb/cockroach#8823 (comment) but a quick discussion concluded that "less options is better". I'd like to revisit that. If I chose to wrap, let it be.
To lawyer up, this rule is not in the style guide. The style guide has vague wording saying to assume code is gonna be read on 100 cols terminals and to wrap at 100, but I'm pretty sure the intent was to set an upper bound.
cc @RaduBerinde @knz @paperstreet
The text was updated successfully, but these errors were encountered: