golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.62k stars 17.49k forks source link

cmd/gofmt: reformats block comments #5128

Open gopherbot opened 11 years ago

gopherbot commented 11 years ago

by borman@google.com:

What steps will reproduce the problem?

Use /* */ for block comments.  Example:

func f() {
        /*
         * don't do this right now
        fmt.Printf(`a
multi-line
string`)
        */
        return
}

Gofmt alters the content of the comment (which includes the /* and */).  The first time
gofmt is called it produces:

func f() {
        /*
                 * don't do this right now
                fmt.Printf(`a
        multi-line
        string`)
        */
        return
}

and if called again it produces:

func f() {
        /*
                         * don't do this right now
                        fmt.Printf(`a
                multi-line
                string`)
        */
        return
}

gofmt should not have altered the comment in the first place.  The /* was indented
correctly up front.
gopherbot commented 11 years ago

Comment 1 by borman@google.com:

Another indentation fail with /* */:
Start with:
/*
        This is a comment
        It is indented with 8 spaces.
        And has several lines
 */
Gofmt changes this to:
/*
   This is a comment
   It is indented with 8 spaces.
   And has several lines
*/
It has changed my 8 spaces into 3.
griesemer commented 11 years ago

Comment 2:

Owner changed to @griesemer.

gopherbot commented 11 years ago

Comment 3 by hamo.by:

The first issue you reported seems a expected output for issue #1835.
griesemer commented 11 years ago

Comment 4:

There is a tension between gofmt leaving comments alone and "adjusting" them to match
surrounding code. There is a know bug where formatting of /* comments is not idempotent
(as witnessed in the first example above), and where formatting doesn't change anymore
after two applications of gofmt (as is also the case in the first example above).
Being completely idempotent is a very hard problem (short of simply running gofmt twice
internally).
Independent of idempotency, the question is what gofmt should be doing with /* comments.
You are suggesting that it should leave it alone if the first line is indented
correctly. Perhaps that is the right answer. But I suspect there are counter examples.
Leaving alone for now.

Status changed to Thinking.

rsc commented 11 years ago

Comment 5:

Labels changed: added priority-later, go1.2maybe, removed priority-triage.

rsc commented 11 years ago

Comment 6:

Labels changed: added feature.

robpike commented 11 years ago

Comment 7:

Leaving alone for 1.2.

Labels changed: added go1.3maybe, removed go1.2maybe.

robpike commented 11 years ago

Comment 8:

Labels changed: removed go1.3maybe.

josharian commented 10 years ago

Comment 9:

Another use for leaving /* comments */ completely untouched is for example output with
whitespace: https://golang.org/issue/6416
rsc commented 10 years ago

Comment 10:

Labels changed: added go1.3maybe.

rsc commented 10 years ago

Comment 11:

Labels changed: removed feature.

rsc commented 10 years ago

Comment 13:

Labels changed: added release-none, removed go1.3maybe.

rsc commented 10 years ago

Comment 14:

Labels changed: added repo-main.

dmitshur commented 9 years ago

Comment 15:

I ran into this issue recently, with an even simpler reproduce case:
http://play.golang.org/p/Pwn4cOHtMN
griesemer commented 9 years ago

Comment 16:

Cool! Thanks.
dmitshur commented 9 years ago

I've done some investigating here.

I ran into this issue recently, with an even simpler reproduce case:

http://play.golang.org/p/Pwn4cOHtMN

That particular issue is due to an unhandled edge case in stripCommonPrefix of go/printer.

It happens when a comment block consists of a first line with /*, 1 or more blank lines, last line with */. The internal prefix variable ends up never being set to a correct value before being used.

Handling it correctly will fix that particular sample I provided (http://play.golang.org/p/Pwn4cOHtMN), but it seems like it's not the same issue as what's described in the original post.

Should I break http://play.golang.org/p/Pwn4cOHtMN out into a separate issue then? Or just consider this current issue to be it?

dmitshur commented 9 years ago

Okay, I've created failing test cases in https://github.com/shurcooL/play/commit/bfc69a3958c14fb662e71a4fe1ef6906b5f818b0 and a simple fix for the issue in https://github.com/shurcooL/play/commit/f416cec7f1cf85d184e8e0581a17c5c636a36a18. I didn't try to refactor it in any way.

I can submit a CL for review as soon as I'm sure if I should say "fixes #5128" or create a separate issue...

dmitshur commented 9 years ago

After re-reading the original issue and the responses it received, I feel that I should move http://play.golang.org/p/Pwn4cOHtMN into a separate, much smaller issue. That way I can proceed to make a CL that completely fixes and closes it. So, I've created #9751.

It has a simple and direct fix. I've made a CL for it.

c0b commented 7 years ago

I have an Example test case:

func ExampleDecode() {
    call-my-test-case it print some output

    // Output:
    /*
here is the output in a block comment
   in multi lines with different indentation
    */
}

The test case is passing, it's no problem

but when I run go fmt, it constantly change the block comment to this style

    /*
    here is the output in a block comment
       in multi lines with different indentation
    */

and causing test case to FAIL;

wonder is it the same case here, is there any way to let go fmt not touching my block comment?

griesemer commented 7 years ago

@c0b There's no way to let go fmt know that it should leave comments alone. In your case, try the use of // comments for the Example output instead (e.g., like in https://golang.org/src/text/scanner/example_test.go).

A future gofmt (version) probably should not touch comments at all, but that has other issues.

vearutop commented 5 years ago

It is a real pain to work with multi-line // output: in example tests. I'm trying to assert YAML value.

Currently I have to paste expected content, add // with hotkey and then replace // to // to make things work.

I would appreciate that gofmt does not touch comments following // output:.

c0b commented 5 years ago

There's no way to let go fmt know that it should leave comments alone

how about to change go fmt to recognize the block comment followed by keywords like Output: this convention is in-use by go test; should be considered a builtin feature of go?

there might be other keywords should be recognized as well, if go fmt doesn't want to recognize all of them, then may create a special comment marker like gofmt: DO NOT TOUCH in the first line of the block comment?

A future gofmt (version) probably should not touch comments at all, but that has other issues.

@griesemer are you aware of any ongoing effort of this future gofmt? can connect related PR to here?

komuw commented 3 years ago

I just spent 1 hour trying to get gofmt to leave alone the // output: of a test example I was working on(https://github.com/golang/go/issues/41980), to no avail; until I searched and came across this issue.

I feel like gofmt re-formatting block comments in the specific case of // output: of a test example should be a separate issue that should be decided on independent of this one. But maybe this is just my frustration showing.

komuw commented 3 years ago

It seems impossible, at least for me, to get the following reproducer test to pass; if you run gofmt before running go test. Because gofmt will always reformat the // output comment. And hence this sounds like a bug to me rather than a feature request.

package main

import (
    "fmt"
)

func Hello() {
    fmt.Printf("Hello \n\nWorld")
}

// 1. https://golang.org/pkg/testing/#hdr-Examples
// 2. https://blog.golang.org/examples
func ExampleHello() {
    Hello()

    // Output:
    // Hello 
    //
    // World
}
madkins23 commented 1 year ago

This may nor may not be a new example.

Using the GoLand IDE it is possible to recognize and color various types of comment lines, e.g. TODO: or NOTE:. Follow-on lines are recognized by an extra space at the beginning (in order to avoid really long lines). For example:

// Note: something something something
//  something else
var x int

This example is formatted to:

// Note: something something something
//
//  something else
var x int

If the object referenced by the comment (in this case var x int) is removed the formatting doesn't happen. It does happen in the "middle" of the comment, not just as the last couple of lines.

I realize that this is peculiar to a specific IDE and therefore of the lowest priority. I just thought that I would mention it as this ticket appears to still be open.

ianlancetaylor commented 1 year ago

@madkins23 What you are seeing there is intentional. See the "Doc Comments" section in the 1.19 release notes (https://go.dev/doc/go1.19).

ianlancetaylor commented 1 year ago

This issue is specific to block comments written with /* ... */. Anything involving // comments is a different issue.

madkins23 commented 1 year ago

It happens with /* ... */ as well:

/*
Note: something something something
 something else
*/
var x int

becomes:

/*
Note: something something something

    something else
*/
var x int

I'm aware of the (IMHO) odd ways that comments are processed by godoc. While I like gofmt in general I didn't expect it to reinterpret and change my comments. But there it is.

Too bad we can't have markdown in comments. Just saying.

ianlancetaylor commented 1 year ago

Too bad we can't have markdown in comments. Just saying.

In case you haven't, see the discussion at https://go.googlesource.com/proposal/+/refs/heads/master/design/51082-godocfmt.md#rationale .

scudette commented 6 months ago

Is it possible to just make go fmt completely ignore block comments with eg two ** as in

/** 
  Leave this alone!
**/

It is quite annoying since I run go fmt as a save hook and recently it started introducing useless reformat changes to copyright blocks particularly ones with indents

balakumarsubramani commented 2 months ago
//nolint:reformat

Works like a charm.

EDIT: