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

completion of import declaration doesn't work on windows. #334

Closed
wants to merge 2 commits into from

Conversation

mattn
Copy link
Contributor

@mattn mattn commented Mar 10, 2016

Fixes #332

@nsf
Copy link
Owner

nsf commented Mar 10, 2016

I don't like the fix.

Maybe you could just replace the usage of "path/filepath" lib with "path"? I think it does POSIX paths by default everywhere.

@mattn
Copy link
Contributor Author

mattn commented Mar 10, 2016

Maybe you could just replace the usage of "path/filepath" lib with "path"?

I just replace backslash into slash because the path separator is always backslash on windows.

@mattn
Copy link
Contributor Author

mattn commented Mar 10, 2016

If you want, I'll add checking of runtime.GOOS.

@mattn
Copy link
Contributor Author

mattn commented Mar 10, 2016

I'm thinking, filepath.ToSlash doesn't define that it works neither doesn't works for partial strings.

@nsf
Copy link
Owner

nsf commented Mar 10, 2016

No, I don't like the fact of replacement. When there is a function above which does concatenation. My suggestion is to fix concatenation so that it does the right thing.

There is a path lib: https://golang.org/pkg/path/

It always uses the right kind of slash. You can just replace filepath usage with it.

@mattn
Copy link
Contributor Author

mattn commented Mar 10, 2016

Okay, however, docs should say it works for partial strings too, I think.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

@mattn does #335 work for you?

@mattn mattn closed this Mar 14, 2016
@mattn mattn deleted the windows-import branch March 14, 2016 06:57
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.

3 participants