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

gf creates files when the given file does not exist #2683

Closed
SuyogSoti opened this issue May 25, 2018 · 7 comments
Closed

gf creates files when the given file does not exist #2683

SuyogSoti opened this issue May 25, 2018 · 7 comments
Labels

Comments

@SuyogSoti
Copy link
Contributor

Is this a BUG REPORT or FEATURE REQUEST? (choose one):
But Report

What happened:
I press gf in normal mode on any keywork, and the extension creates a file with the name as that keyword in the directory.

What did you expect to happen:
for the extension to not do anything. Please do not make it give me an error saying the file does not exist.

How to reproduce it (as minimally and precisely as possible):
Go to any word in your code base and press gf for a file that does not already exist.

Environment:
Manjaro

  • Extension (VsCodeVim) version: 0.12.0
  • VSCode version: 1.23.1
  • OS version: Manjaro
@jpoon
Copy link
Member

jpoon commented May 25, 2018

I don't quite understand the behaviour you expect. We do a best effort in trying to find the file (based off your relative path and/or workspace directory). However, if the file does not exist, you want the extension to do nothing? Don't create the file, and don't show an error? IMO that behaviour seems rather unfriendly to a user as there's no feedback to the user after command execution.

In any case, our original behaviour was to show an error dialog, but there was a relatively popular ask to automatically create the file which is the behaviour you see today #2274. For that reason, I'm inclined to close this bug as by-design.

@SuyogSoti
Copy link
Contributor Author

I do agree that #2274 is a great idea but I think that there should be a difference between how :e and gf operate. If possible I think that gf should only go to the file if it exists and :e should create the file if it does not exist.

As for the notification, I think that it would be enough to the user to see that they did not go to a new file to see that that file does not exist. If there should be a notification, I think that it should be done in a more discrete way. The notification system in vscode is too distracting. To be perfectly honest, my request for the no notification more secondary to my request that gf should not open a new file.

@SuyogSoti
Copy link
Contributor Author

Hey,

I just wanted to check in with you about what you think about the gf creating new files if it does not exist. I did just a quick look in the code base and it seems like creation of the new file happens in src/cmd_line/commands/file.ts in the class FileCommand on line 105: fs.closeSync(fs.openSync(filePath, 'w')).
A notification there instead of a new file would work great.
I checked and I see that this is the only place that FileCommand is being used.

What I am trying to get at is I just want to check in about what we think about it not creating new files. If this is an issue of time, this seems like a simple change where I am more than happy to create a pull request. But if this is an issue of the design principle, then I would just loved to be looped in.

@jpoon
Copy link
Member

jpoon commented May 31, 2018

That code path is also shared by the :e operation. So if you wanted a separate behaviour for gf, you'll need to distinguish how that function was called.

Can you help me understand your scenario a bit? Once you see that notification, what do you plan to do afterwards?

@SuyogSoti
Copy link
Contributor Author

Ok so in the perfect world, here is what would happen.

:e somefile

would create some file.

gf existing file

would take me to existing file

gf non-existing file

would give me a notification and not do anything just like what vim currently does. The user at that point can decide to individually create that non-existing file or not. The current behaviour is that gf just creates that non-existing file. I do not think that is good. gf should not do the same thing as :e. In vim gf does not do the same thing as :e.

If the FileCommand is shared by :e as well then we can just have an optional arguement in the dictionary that is passed in to create the file or not.

@jpoon
Copy link
Member

jpoon commented Jun 1, 2018

If the FileCommand is shared by :e as well then we can just have an optional arguement in the dictionary that is passed in to create the file or not.

I'm fine with that if you want to submit a PR :)

jpoon pushed a commit that referenced this issue Jun 7, 2018
* fix gf to be like issue #2683

* converted to vscodevim style

* fix bug that came with variable double negation
@jpoon jpoon added the kind/bug label Jul 15, 2018
@johnyoonh
Copy link

Would it be make changes so that createFileIfNotExists can be toggled in settings.json?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants