search

simplenote-vim / simplenote.vim

vim plugin to interact with the simplenote service
http://www.vim.org/scripts/script.php?script_id=3582
MIT License
368 stars 31 forks source link

prompt to input user/pass when missing #69

Closed yuex closed 9 years ago

yuex commented 9 years ago

if no username or password is pre-configured, will prompt user to input. And the authentication moved in simplenote#SimpleNote(), thus the plugin won't explode if you interrupt your first login and want to login again or your login failed.

atomicules commented 9 years ago

Hi. Thanks for this. I will review it and update back here in a couple of days time.

atomicules commented 9 years ago

Sorry, will probably be the weekend now when I get around to reviewing this.

m1foley commented 9 years ago

I just tested this and it works as expected.

atomicules commented 9 years ago

Thanks for testing. I still need to make some tweaks to either my Vader tests or this pull request though so my tests still run ok.

I just don't have the time in the evenings this week. Will be able to get around to it at the weekend though.

atomicules commented 9 years ago

Hi. So I did get around to looking at this over the weekend. I can't merge it as is, because it breaks current functionality, it seems to reset/ignore the g:SimplenoteUsername after a point.

However, I do like the idea and the idea of using SimplenoteLoginFailed so I'm going to carry on working on this to get it to something that can be merged in, likely to be anywhere between a few days to a few weeks.

When entering a username/password, how were you intending logon failures to work? Do you want to be continually prompted for username/password until it is correct? The user could always CTRL+C out of it.

yuex commented 9 years ago

@atomicules I changed the code. reset_user_pass() now will reset respectively s:user and s:password to g:SimplenoteUsername and g:SimplenotePassword

In the PR, when logon failed, a warning message will be printed, and the function will return. But the user is free to launch :Simplenote and try again.

Personally I prefer not retrying automatically (blindly, perhaps). in most cases it may be considerate, but wehn I don't want to retry, it really feels presumptuous and officious.

atomicules commented 9 years ago

Hi. I've just pushed some minor tweaks to your pull request to a temporary branch, just so you have a chance to review before I close this out. The changes I made are explained in the commit message and I tried to keep as much as your code as possible since you've gone to the trouble to write it.

The only thing I might think about before closing this out is if there is anyway to prevent the Simplenote buffer from being created if a login exception occurs. I don't think there is a clean way though as the buffer is created first. It's not a huge deal either.

Oh, I'll also have to update the README before closing out to mention this new functionality.

atomicules commented 9 years ago

I think this is pretty much ready to close out. I've had a look into the buffer thing and I think what I really need to do is to change simplenote.py so during initialisation it tries to fetch a token straight away. That way we can catch the SimplenoteLoginFailed as early as possible (i.e. in initialisation of SimplenoteVimInterface). But that's a job for another day.

yuex commented 9 years ago

looks good to me. I'll close this PR right now but feel free to re-open it

atomicules commented 9 years ago

I've merged all that into master now. Thanks for your contribution!