incu6us / goimports-reviser

Right imports sorting & code formatting tool (goimports alternative)
MIT License
585 stars 75 forks source link

Support for passing gofmt options #34

Open jdechicchis opened 3 years ago

jdechicchis commented 3 years ago

It would be great to have the ability to pass gofmt options such as -s etc. Looking at https://golang.org/src/cmd/gofmt/ not sure what the best approach would be here. I see goimports-reviser is currently using go/format. Ideally, goimports-reviser wouldn't have to duplicate the work of gofmt, including it's flags, but I also see how calling the gofmt binary may not be desired as well. Happy to make a PR, but would appreciate some feedback since I'm not very familiar with this.

incu6us commented 3 years ago

Hi @jdechicchis, Actually goimports-reviser is created for imports(but we can expand functionality) and yes, it uses go/format to make it possible to perform basic cleanup for the code using public functionality of the library. gofmt encapsulates their methods for -s option and also has BSD license with which we can't just use the implementation. Include the gofmt as a binary into the project is also not a good solution, I believe. I propose you create a list of the functionality to perform cleanup and simplicity for the code and start the discussion in this direction. So, what should be done?

jdechicchis commented 3 years ago

I see, that makes sense. The licensing is a hurdle. We were thinking that since goimports-reviser does some basic formatting, adding functionality to totally replace the need to call gofmt would be nice. I'd be happy to iron out the requirements more and dive into the code but it's also up to you if you'd want formatting functionality in goimports-reviser which would have to be maintained independently of gofmt going forward.

P.S. the actual import sorting functionality is great :)

incu6us commented 3 years ago

@jdechicchis Thanks:)

Ok. Let's discuss the requirements. Here are some ideas which could be start point:

If you have an idea, then welcome:) Let's discuss them and convert then into tickets/issues

jdechicchis commented 3 years ago

Apologies for the delayed reply. Those make sense to me. Perhaps we can also look to gofmt's simplify option as a starting point? Even though we can't copy their implementation, I think mirroring the functionality (although someone tedious) may make for a nice addition to goimports-reviser. From their docs:

An array, slice, or map composite literal of the form:
    []T{T{}, T{}}
will be simplified to:
    []T{{}, {}}

A slice expression of the form:
    s[a:len(s)]
will be simplified to:
    s[a:]

A range of the form:
    for x, _ = range v {...}
will be simplified to:
    for x = range v {...}

A range of the form:
    for _ = range v {...}
will be simplified to:
    for range v {...}

I suppose one could add a -format option to goimports-reviser to tune the additional formatting with the goal of mirroring (and maybe exceeding!) what gofmt does.

incu6us commented 3 years ago

@jdechicchis Cool, I like it! I'll create separate issues for that(perhaps today-tomorrow) with the label need help and we could start with the implementation. This ticket still be opened to discussing general questions for -format option.

incu6us commented 3 years ago

@jdechicchis I've created issues about formating option #36, #37, #38, #39 and #40(issue #35 is a general issue and attached to separated milestone). So, you can pick any of them.

jdechicchis commented 3 years ago

Sounds great @incu6us! I'm hoping to pick up some of these issues starting next week

incu6us commented 3 years ago

35 - done

incu6us commented 3 years ago

36 - done