search

rafaqz / citation.vim

Zotero and bibtex citations for Vim
MIT License
96 stars 7 forks source link

Fixing raiseError() function #12

Closed DancingQuanta closed 7 years ago

DancingQuanta commented 7 years ago

There was a undefined variable in raiseError() function in python/citation_vim/ultis.py and some instances of raiseError() are inconsistent in rest of the codebase.

This pull request changes the function to only one line where a string message is passed to RuntimeError with Citation.vim error: string appended to the message and changed all instances of raiseError() to accept one string.

DancingQuanta commented 7 years ago

Oops, my vim appears to remove quite a lot of white spaces from the files I visited....

rafaqz commented 7 years ago

Whoa there. raiseError prints *args - it's meant to be inconsistent!

But there was a missing variable in the error, thanks for picking that up. It's mostly academic as an error is raised anyway and already printed, but fixed!

DancingQuanta commented 7 years ago

I used $HOME instead of ~ in cache path. However that was in my git history as I was trying to track down why I am getting an error in clean() where I found sometimes a string is actually an integer.

On Friday, June 16, 2017, rafaqz notifications@github.com wrote:

Closed #12 https://github.com/rafaqz/citation.vim/pull/12.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rafaqz/citation.vim/pull/12#event-1126229272, or mute the thread https://github.com/notifications/unsubscribe-auth/AIB3Vc-cTnyU8KcDA9T3nMsU8Tpn_DuOks5sEgcLgaJpZM4N7ZJ5 .

-- Andy Tolmie

rafaqz commented 7 years ago

Sure, the error handling it does seem kind of inconsistent really looking through it, I'll merge and pull the change. Honestly the pull request just seemed messy with the whitespace deletion etc, and you deleted my pet obscure *args passing for a more generally sane format option!! horror. Cheers

DancingQuanta commented 7 years ago

I am sorry about the whitespace, I did not have my vim beast under control. Would it be possible to patch merge and keep your whitespace?

On 16 Jun 2017 8:23 a.m., "rafaqz" notifications@github.com wrote:

Sure, the error handling it does seem kind of inconsistent really looking through it, I'll merge and pull the change. Honestly the pull request just seemed messy with the whitespace deletion etc, and you deleted my pet obscure *args passing for a more generally sane format option!! horror. Cheers

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rafaqz/citation.vim/pull/12#issuecomment-308953809, or mute the thread https://github.com/notifications/unsubscribe-auth/AIB3VfsLwMVWFhXgJtT7oKixHgDoeiyBks5sEi2HgaJpZM4N7ZJ5 .

rafaqz commented 7 years ago

Thought that might be the case... I turned off whitespace cleanup after a protracted edit war with a colleague whos setup auto-added eof returns. Anyway I realised the merge doesn't make sense as error handling needs a total cleanup, there are relict nested try/except statements in there.

Basically the whole plugin had to be put in a try/except block to force vim/unite to actually print python errors instead of failing silently. See here for the full horror of it all: https://github.com/rafaqz/citation.vim/blob/master/python/citation_vim/citation.py

DancingQuanta commented 7 years ago

Hmm, need to check with Unite.vim to see if there is any way to better error handling?

On Fri, Jun 16, 2017 at 10:12 AM, rafaqz notifications@github.com wrote:

Closed #12 https://github.com/rafaqz/citation.vim/pull/12.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rafaqz/citation.vim/pull/12#event-1126497339, or mute the thread https://github.com/notifications/unsubscribe-auth/AIB3VcQeOtJGAsDNj3Wk7UzfGG916BUDks5sEkb9gaJpZM4N7ZJ5 .

-- Andy Tolmie

DancingQuanta commented 7 years ago

Thankfully, my current configuration is working but this plugin's error handling need reconsidering.

On Fri, Jun 16, 2017 at 10:17 AM, Andrew Tolmie andytheseeker@gmail.com wrote:

Hmm, need to check with Unite.vim to see if there is any way to better error handling?

On Fri, Jun 16, 2017 at 10:12 AM, rafaqz notifications@github.com wrote:

Closed #12 https://github.com/rafaqz/citation.vim/pull/12.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rafaqz/citation.vim/pull/12#event-1126497339, or mute the thread https://github.com/notifications/unsubscribe-auth/AIB3VcQeOtJGAsDNj3Wk7UzfGG916BUDks5sEkb9gaJpZM4N7ZJ5 .

-- Andy Tolmie

-- Andy Tolmie

rafaqz commented 7 years ago

Ok, pull the latest, mess some variable up, post the error and explain your issue with it. I'm actually keen to make this make sense. Some of your code made it in (ie message arg and format), but not the merge unfortunately.

If there exists an easier and and saner way of printing all possible python errors to the console through the unite/vim stack I will use it.