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

Open inexisting files #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Open inexisting files #4

wants to merge 2 commits into from

Conversation

DonRyuDragoni
Copy link

When opening a file that does not already exist, it is expected that the editor opens a blank file. Vy does no such thing (instead, it crashes).

I'm not entirely sure this is the best solution, but it is sure to create the new file or open an existing one. Since I'm also checking if filename is a file (and not a directory), this may lead to another bug where a file with the same name of the directory is created. I'm not sure at the moment how to fix this nor if it in fact may happen.

Also, changed the opening of files. Previously, it was

file = open(filename, mode)
file.read()
file.close()

this can be changed to the more pythonic

with open(filename, mode) as f:
    f.read()

that will also handle f.close(). This change, however, is purely aesthetic and should not compromise the program's behavior.

DonRyuDragoni added 2 commits September 3, 2015 14:46
The previous code would make the program crash if the file did not
already exist before the program started. Now, if it does not, it will
create it (may be inconvenient).

Also, changed to a more pythonic form.
Changed the "open(); read(); close()" instructions to the more simple
and pythonic "with open(filename, 'w') as f: (...)".
@iogf
Copy link
Member

iogf commented Sep 3, 2015

what if the directory doesnt have permissions for writting? it is not the case that one may always want to create a file when running vy. suppose he just want to use the terminal-like plugin to type some commands? although it may be the case one want to create a file, in that case a good solution would add a command option like vy --create-file filename. what do you think?
I agree that using with statement is aesthetically better.

@DonRyuDragoni
Copy link
Author

You do have a point, but vi/vim, emacs, sublime and others reflect this behavior, and the user is responsible for handling permissions. Maybe some warning in this case will do? Something on the lines of "Hey, I'm fine with opening this file, but it looks like you do not have the right to write it."?

Also, I was thinking. These editors also won't immediately create the file, only when the user asks for a save. Maybe just getting rid of the else entirely and opening only in the if os.path.exists (...) is a better choice?

@iogf
Copy link
Member

iogf commented Sep 3, 2015

if you press it creates the file regardless if it exists or not. if you do
vim eee
it will not create the file at all, only if you save it. but it opens a tab named
eee

@iogf
Copy link
Member

iogf commented Sep 3, 2015

consider this situation as well.

vy /some_dir_that_doesnt_exist/some_file_that_doesnt_exist

it would lead to more complex situations like? does the user want to create that dir?
why should an editor create new dirs?

@iogf
Copy link
Member

iogf commented Sep 3, 2015

the basic idea in leaving that exception be thrown consists of giving a slight idea to the user why it couldnt open the file. vy is meant for people who like programming, they would be familiar with the exception message.

@DonRyuDragoni
Copy link
Author

Yes, I knew that vim would not create the new file, only on save. That's why I asked to get rid of the else part. I suggested vy to still "open" the file, just not write it.

In the case of an unexisting directory, the program could just say that it can't write the file because the directory does not exist. Without a crash. In this case, it is totally valid that the program simply doesn't write the file and spits an error message in a message box or something alike.

What I'm trying to say is that I understand your point, but an editor that crashes on a simple task of creating a new file seems... wrong. An odd behavior, at least. Yes, people could be familiar with the message (and with Python), at least enough to understand its meaning, but but it carries the feeling that the vy is defective. I cannot think of this behavior as something intended, rather than a known bug.

@iogf
Copy link
Member

iogf commented Sep 3, 2015

well. if you have such a feeling then other people would have that feeling so i would agree it demands a message informing about it.

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.

2 participants