Gofmt proposal for ignore construct

[Иван Перевезенцев]

Automatic code formatting is not trivial task to implement and there are many cases when it does exactly inverse thing and makes code less readable, for example see this issue.

One can apply diffs from gofmt by hand, but it consumes time and is not fun. This problem can be easily solved if gofmt would recognize special comments like "// gofmt on", "//gofmt off". What do you think about that?

[Ian Lance Taylor]

Personally I think it would be a bad idea. One of the main points of
gofmt is to eliminate discussions about coding style. Supporting
those comments would bring those discussions back. The cost exceeds
the benefit. gofmt doesn't have to be perfect; it just has to be good
enough.

Ian

[Paul Borman]

I played around a bit with what you were trying to do (comment parts of the string).  Taking your original code:

const REGEXP_WHOLE =

// [1] Return value

`^\s*` + REGEXP_TYPE +

// 3+1 = [4] Function name

`(gl\w+)\s*` +

// [5] Argument list

`\((` + REGEXP_ARG_LIST + `)\)\s*$`

I ended up with

const (

returnValue  = `^\s*` + RegexpType               // [1]

functionName = `(gl\w+)\s*`                      // 3+1 = [4] Function name

argList      = `\((` + RegexpArgList + `)\)\s*$` // [5] Argument list

RegexpWhole  = returnValue + functionName + argList

)

It actually is one line shorter.  I am not sure if you would be happy with this.  It is not exactly what you had, but it plays relatively nicely with gofmt.

I agree with Ian  // gofmt off would defeat the purpose of gofmt.  I can imagine people just making that the first comment after their package statement...

    -Paul

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Иван Перевезенцев]

Regarding the costs. The purpose of such feature is not to introduce ambiguity, but to allow preserve such (not uncommon) cases that gofmt handles improperly. Unfortunately at the moment it is not good enough for many of such cases. Look at the output of gofmt in example from my first message. I suppose it is not the ultimate convenient "coding style".

пятница, 21 августа 2015 г., 17:50:15 UTC+3 пользователь Ian Lance Taylor написал:

[Ian Lance Taylor]

On Fri, Aug 21, 2015 at 8:07 AM, Иван Перевезенцев
<ivan.pere...@gmail.com> wrote:
>
> Regarding the costs. The purpose of such feature is not to introduce
> ambiguity, but to allow preserve such (not uncommon) cases that gofmt
> handles improperly. Unfortunately at the moment it is not good enough for
> many of such cases. Look at the output of gofmt in example from my first
> message. I suppose it is not the ultimate convenient "coding style".

I understand that the purpose is not to introduce ambiguity.
Nevertheless, that is what it would do. Therefore, I believe there
would be a cost, and I believe that cost is too high for the benefit.

Ian

[Иван Перевезенцев]

Ok, for this case introducing three new constants just to keep it in one line works not bad.

But it still corrupts other class of cases:

returnType  := submatches[1] // before
funcName    := submatches[4]
argList     := submatches[5]

returnType := submatches[1]    // after
funcName := submatches[4]
argList := submatches[5]

Again, about defeating the purpose of gofmt, please see previous message. It makes sense in small distinct parts of code. If somebody will use "// gofmt off" at the beginning of file and right to the end, it's unlikely to such code to be popular regarding low skill level of it's author, because this is obvius misuse. And what's the point to run gofmt in this case at all?

пятница, 21 августа 2015 г., 17:54:52 UTC+3 пользователь Paul Borman написал:

[Roberto Zanotto]

To avoid discussions about formatting like this one :D

[Иван Перевезенцев]

I just remembered about command line options. A great solution that would satisfy everyone's needs is to make it optional and turned off without a specific flag.

I believe it wouldn't make any practices worse because gofmt is still an optional tool, package maintainers create and share code considering it's usage or not.

Conversely, lack of ability to locally tell it to make exception exactly encourages people to avoid use of gofmt, because process of formatting your files semi-manually by comparing diffs takes unnecessary efforts. I want just hit Ctrl-S and done. So such feature would be really worth it.

There is the case that some team's build environment use automatic gofmt and nobody review the code so all-file "// gofmt off" may leak in (can't imagine such a dumb), but remember - this feature would be off by default, unless you specify a flag.

пятница, 21 августа 2015 г., 18:11:38 UTC+3 пользователь Ian Lance Taylor написал:

[Иван Перевезенцев]

We can also show big screaming banner if i.e. 50% of file was commented, probably erasing the hard drive.

[Egon]

On Friday, 21 August 2015 18:55:10 UTC+3, Иван Перевезенцев wrote:

I just remembered about command line options. A great solution that would satisfy everyone's needs is to make it optional and turned off without a specific flag. 

I believe it wouldn't make any practices worse because gofmt is still an optional tool, package maintainers create and share code considering it's usage or not.

Conversely, lack of ability to locally tell it to make exception exactly encourages people to avoid use of gofmt, because process of formatting your files semi-manually by comparing diffs takes unnecessary efforts. I want just hit Ctrl-S and done. So such feature would be really worth it.

Why cannot you do it right now? You can ignore spacing changes in diffs.

I would say that yes the formatting is not perfect, but it's not hindering either, at least in most code.

My suggestion would be:

run go fmt

ignore minor formatting issues

move on to fixing bugs or adding features

At the end of the day, perfect code formatting has little value to the end-user. Worrying about it and manually fixing it, wastes time.

go fmt gives good enough results so you wouldn't need to waste time.

After a while you can read such code just as well.

+ Egon

[Иван Перевезенцев]

Of course I could just fix it myself by hand and leave it. But there are people running into this commonly. I am bothering because I want Go to have polished tools and provide smooth workflow. Formatting doesn't matter to end user, but matters for maintainability. Here is another example, consider I want to embed in code a chunk of binary data: http://play.golang.org/p/HQ5edXhdtS

пятница, 21 августа 2015 г., 19:34:29 UTC+3 пользователь Egon написал:

[Egon]

On Friday, 21 August 2015 20:11:46 UTC+3, Иван Перевезенцев wrote:

Of course I could just fix it myself by hand and leave it. But there are people running into this commonly. I am bothering because I want Go to have polished tools and provide smooth workflow. Formatting doesn't matter to end user, but matters for maintainability. Here is another example, consider I want to embed in code a chunk of binary data: http://play.golang.org/p/HQ5edXhdtS

I would agree that it should be aligned by go fmt, there is an open issue regarding matrix alignment: e.g. https://github.com/golang/go/issues/8974.

I would suggest opening a new issue for that case, since https://play.golang.org/p/NYV9i7bSA-

Although there might be other reasons why it's not aligning those comments.

The gofmt is definitely not set in stone and there will be fixes and modifications made to it.

The one thing that won't be included is customization -- it gets us back to the "wasting user time", even if it's optional... basically there should be one style to rule them all.

In that matrix case, if you are writing those bytes and comments manually there's also a possibility of using a custom function: http://play.golang.org/p/E27cOxqeh9. (you had several bugs in your comments, hence using additional validation)

Although, in all honesty, I cannot fathom why you would need those comments there anyways, if you are generating that code. But, I can imagine you would need comments in some triangle matrix, where it would be nice if they are aligned.

+ Egon

[Giulio Iotti]

On Friday, August 21, 2015 at 9:00:40 PM UTC+3, Egon wrote:

On Friday, 21 August 2015 20:11:46 UTC+3, Иван Перевезенцев wrote:

Of course I could just fix it myself by hand and leave it. But there are people running into this commonly. I am bothering because I want Go to have polished tools and provide smooth workflow. Formatting doesn't matter to end user, but matters for maintainability. Here is another example, consider I want to embed in code a chunk of binary data: http://play.golang.org/p/HQ5edXhdtS

In that matrix case, if you are writing those bytes and comments manually there's also a possibility of using a custom function: http://play.golang.org/p/E27cOxqeh9. (you had several bugs in your comments, hence using additional validation)

I'd also propose this, if it doesn't look to stupid: https://play.golang.org/p/nfJJdIFdd4

-- 

Giulio Iotti

https://twitter.com/dullboy

[Michael Jones]

I tend to do a get creative with extra comments to preserve alignment, as below, and often include code blocks twice, once in comments with careful alignment and once plain.

type expectedSymmetry struct {

//      // Note: OEIS sequences referenced here are shifted inconsistently at OEIS...aligned terms below:

n11 int // 1, 0, 0, 1,  1,  0,   0,   1,    2,    0,     0,     3,      2,      0,       0   OEIS A142886

n21 int // 0, 0, 0, 0,  0,  0,   0,   1,    0,    0,     0,     3,      2,      0,       0   OEIS A144553

n12 int // 0, 1, 1, 1,  1,  2,   4,   5,    4,    9,    12,    18,     20,     35,      41   OEIS A056877 + OEIS A056878

n22 int // 0, 0, 0, 1,  1,  5,   4,  18,   19,   73,    73,   278,    283,   1076,    1090   OEIS A006747

n14 int // 0, 0, 1, 1,  4,  8,  16,  28,   64,  112,   238,   420,    890,   1595,    3334   OEIS A006746 + OEIS A006748

n24 int // 0, 0, 0, 1,  5, 20,  84, 316, 1196, 4461, 16750, 62878, 237394, 899265, 3422111   OEIS A006749

//      // -------------------------------------------------------------------------------

//      // 1, 1, 2, 5, 12, 35, 108, 369, 1285, 4655, 17073, 63600, 238591, 901971, 3426576   OEIS A000105

}

— 

Michael Jones, CEO  •  mic...@wearality.com  •  +1 650 656-6989 

Wearality Corporation  •  289 S. San Antonio Road  •  Los Altos, CA 94022

[xiio...@gmail.com]

On Friday, 21 August 2015 16:55:10 UTC+1, Иван Перевезенцев wrote:

I just remembered about command line options. A great solution that would satisfy everyone's needs is to make it optional and turned off without a specific flag.

This seems like a good idea - the base of the issue in this case is whether or not to indent the first line as the following lines .. a matter of presentation

Why not allow users to set their own prefs for gofmt but still require gofmt - a single build converts to the desire presentation anyway..

Could get go get to automatically gofmt'ed code to the uses preference (if it doesn't anyway..)