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

better deal from command line when file is not found #13

Open
gutomaia opened this issue Apr 27, 2014 · 8 comments
Open

better deal from command line when file is not found #13

gutomaia opened this issue Apr 27, 2014 · 8 comments
Assignees
Milestone

Comments

@gutomaia
Copy link
Owner

At the command line when calls: "pynes asm example.asm -o example.nes", you receive an ugly stack trace on the file doesn't exists. User must receive a better error handle in that case.

@gutomaia gutomaia added this to the pyNES 0.1.3 milestone Apr 27, 2014
@danilobellini
Copy link
Collaborator

This should be quite simple, calling parser.error after a os.path.isfile verification (which also returns False when the file doesn't exist, so there's no need for calling os.path.exists explicitly). It would be somewhat similar to what was done here.

You can also use the type=argparse.FileType("r"), but that already opens the file and I don't know how it ensures that the file is closed afterwards, if that's really enforced by it.

@gutomaia
Copy link
Owner Author

@danilobellini We can give a try. I mean, I found two errors due to a file not found.

@popmilo what do you think about this task? @danilobellini could also help you and discuss possible alternatives, however he is already engaged in another task.

@popmilo
Copy link
Collaborator

popmilo commented Apr 27, 2014

Yeah, I can do that. I'll start working on it.

@popmilo
Copy link
Collaborator

popmilo commented Apr 29, 2014

Finally managed to spend some more time reading main code, really well written and interesting.
Here is just a quick fix for this issue. Is it ok in this way (check if input file is valid and continue if it is) ?

   args = parser.parse_args(argv[1:])
   if is_valid_file(parser, args.input):
        args.func(args)

def is_valid_file(parser, arg):
    if os.path.isfile(arg):
        # File exists
        return True
    else:
        parser.error('The file {} does not exist!'.format(arg))
        return False

This is the result of "pynes asm example.asm -o example.nes":
"usage: pynes [-h] {py,chr,asm,nt,img} ...
pynes: error: The file example.asm does not exist!"

Should there be a check for output file name validity also ?

@danilobellini
Copy link
Collaborator

This can be slightly simpler, since the parser.error already terminates the program.

@popmilo
Copy link
Collaborator

popmilo commented Apr 29, 2014

You are right :)
This piece of code inserted before "args.func(args)" does the job:

    if not os.path.isfile(args.input):
        parser.error('The file {} does not exist!'.format(args.input))

@popmilo
Copy link
Collaborator

popmilo commented Apr 29, 2014

@gutomaia Should I make a pull request for just this one small commit, or work on something else ?
ps. I never thought I'll read and learn this much about Git when I found out about pyNes :)

@gutomaia
Copy link
Owner Author

@popmilo I've just take a look on that, it look's nice. However I, to help improve the community and the software also, the best approach to solve a request is to write a test.

What you have done so far is great. Problem is, if someone like me, break your code. No one will notice 'cause there is tests related to that part.

On the project there is a commandline_test.py, first take a look on it. Try to figure out what test would break the application. If you need any help, just ask! I'm just trying to lead you into a path, not just give you the answer!

Good luck!

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

No branches or pull requests

3 participants