Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include keyify support #1258

Merged
merged 5 commits into from
May 5, 2017
Merged

Conversation

thalesmello
Copy link
Contributor

Includes support for https://github.com/dominikh/go-tools/tree/master/cmd/keyify
through the GoKeyify command.

It turns unkeyed struct literals into keyed struct literals.

Includes support for https://github.com/dominikh/go-tools/tree/master/cmd/keyify
through the GoKeyify command. Turns unkeyed struct literals into keyed struct literals.
plugin/go.vim Outdated
@@ -21,6 +21,7 @@ let s:packages = [
\ "github.com/fatih/gomodifytags",
\ "github.com/zmb3/gogetdoc",
\ "github.com/josharian/impl",
\ "github.com/dominikh/go-tools/tree/master/cmd/keyify",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a wrong import path, should be: github.com/dominikh/go-tools/cmd/keyify

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Changed it.

@fatih
Copy link
Owner

fatih commented May 4, 2017

Hi @thalesmello

I just tried it. The example I've used is:

package main

import "fmt"

type Foo struct {
	foo string
	bar string
	qux string
}

func main() {
	f := Foo{"foo", "bar", "qux"}
	fmt.Printf("f = %+v\n", f)
}
  1. When I put my cursor on top of f or before Foo nothing happens. I think we should support that case, not sure if we can do it without adding support to keyify. If not we should return a message saying that the cursor is not on top of the struct. keyify already does it, we should display it.

  2. If I put my cursor on Foo it converts it to:

package main

import "fmt"

type Foo struct {
	foo string
	bar string
	qux string
}

func main() {
	Foo{
		foo: "foo",
		bar: "bar",
		qux: "qux",
	}
	fmt.Printf("f = %+v\n", f)
}

As you see it removes the variable all together. I assume the replacement is not done correctly.

@thalesmello
Copy link
Contributor Author

thalesmello commented May 4, 2017

@fatih

I think we could implement some kind of mechanism to try to figure out the position of the struct, however, I can't think of a good way of implementing it in a good, clear way.

But I totally agree that it at least should output some nice error message.

In my first implementation, I simply did nothing when I keyify didn't return a refactor.

What I did, then, was simply return keyify's error message, which is actually pretty useful.

Now it returns

Error detected while processing function go#keyify#Keyify:
line   18:
no composite literal found near point

When it doesn't find any struct. What do you think about this?

Hum, strange. It work correctly for me.

A common issue with keyify is that it has to run inside the $GOPATH/src folder, otherwise it isn't able to work.

Could you please check my changes?

@fatih
Copy link
Owner

fatih commented May 4, 2017

When it doesn't find any struct. What do you think about this?

Do not use echoerr. Instead use our own echo messages as echoerr is not user friendly. Here is an example:

    call go#util#EchoError(s:chomp(output))

A common issue with keyify is that it has to run inside the $GOPATH/src folder, otherwise it isn't able to work.

How do we know that it doesn't work. Does it return an error? I'm always inside a GOPATH. vim-go also has with Go 1.8 a default GOPATH as well.

Hum, strange. It work correctly for me.

The problem occurs if you previously have selected anything. For example do a line seletion via "shift-v" and then call :GoKeyify when your cursor is on Foo, you'll see it'll replace with the wrong selection.

@thalesmello
Copy link
Contributor Author

@fatih

I changed the echo function used.

About the bug, here is what happened:

  1. You used a "V" visual mode
  2. GoKeyify relies on using a visual range in order to replace some text
  3. Because the previous visual mode was "V", it's used when you type "gv"
  4. GoKeyify replaces the entire lines content because of this

In order to prevent this bug, now the function checks if the correct visual mode is being used.

Could you check this last version, please?

@fatih fatih merged commit 1a7887e into fatih:master May 5, 2017
@fatih
Copy link
Owner

fatih commented May 5, 2017

Thanks @thalesmello this looks now good 👍

@fatih fatih mentioned this pull request May 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants