-
Notifications
You must be signed in to change notification settings - Fork 104
Fixes #61: Catch yapf errors instead of overwriting file #62
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
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
📝 Please visit https://cla.developers.google.com/ to sign.
Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.
- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
- If you signed the CLA as a corporation, please let us know the company's name.
Just signed the CLA.
CLAs look good, thanks!
autoload/codefmt.vim
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you drop this reassignment while you're in here? AFAICT it serves no purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, didn't notice that. Done.
This is actually wrong, and I don't think I'm familiar enough with vimscript to fix it. Specifically, yapf returns error code 2 if the error is changed, which means I have to catch the ShellError exception, then check if the error code is 2, and do the same formatting logic as in the try. Something like
try
let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call()
catch /ERROR(ShellError):/
if v:shell_error == -1
call maktaba#error#Shout('Error formatting file: %s', v:exception)
endif
endtry
let l:formatted = split(l:result.stdout, "\n")
call maktaba#buffer#Overwrite(1, line('$'), l:formatted)
doesn't work since l:result is scoped to within the try block... I don't know how to declare result to have scope outside the try block in vimscript.
Update: for now, I just got rid of the try catch and checked the v:shell_error variable manually.
yapf only returns an error code of 0 if it does not modify the file. If it does modify the file, it returns error code 2, which causes maktaba to throw a ShellError. Now, we just manually check the error code instead of attempting to catch the exception.
Does this look good? I've been using it for the past few days and it seems to work fine.
Yep, looks good. Thanks!
Fixes #61: Catch yapf errors instead of overwriting file
This isn't perfect, since it doesn't try to parse the yapf output, but it at least won't overwrite the file if there's an error.