search

n0v1c3 / vira

Create and update your Jira issues while inside Vim!
MIT License
94 stars 12 forks source link

Fix :ViraBrowse in Neovim #40

Closed jamesl33 closed 4 years ago

jamesl33 commented 4 years ago

Neovim doesn't support running a command in the built in terminal using '++close'. By default the 'open' command will now be used to open the current issue in the default browser. Users can still override which browser is used by using 'g:vira_browser'.

n0v1c3 commented 4 years ago

@jamesl33 thanks. I am going to have a good look at this. We do have and are building a make sure it works on Vim and nVim testing script.

I should be on it this evening (I'm my time). I will need to do a double check but I don't think browser testing has made it there yet.

If anything comes up I will send you some questions/comments otherwise I will simply need to check the full impact for version control. Luckily we are still calling ourselves Beta 😉

jamesl33 commented 4 years ago

@n0v1c3 no problem at all, thank you for sharing Vira. Sounds good to me, I'm more than happy to update/modify the change from any comments provided.

n0v1c3 commented 4 years ago

@jamesl33 I should get the actual work done "today".

The code is the way it is as @mikeboiko and myself are always in vim and made it possible to browse with async (thanks as always @mikeboiko ). I will do the actual tests and I need to keep Vim and nVim in mind for browser.

I will not deny that usually when I NEED to go to my browser we are missing something in Vira and that is where my focus likes to go. Management is moving up and find the missing pieces will always be part of the fun.

n0v1c3 commented 4 years ago

I have done the merge on Branch VIRA-211, I am currently running this by the other Vim and nVim users/usage that I can and make sure it works then I will move it to the master branch.

n0v1c3 commented 4 years ago

@jamesl33 I have created what looks like should be close to the final answer. My only true issue is I do not have an OSx to truly test it on.

My dev branch currently holds the update if you want to merge it into your master.

jamesl33 commented 4 years ago

@n0v1c3 Sounds great, if you've got a solution you're happy with already in your dev branch I'm happy for you to merge that one and we can just close this pull request with a comment stating that this has been fixed in master.

n0v1c3 commented 4 years ago

@jamesl33 don't worry I took the pull request it just happened to be from my terminal. It did lead to a few changes and then a few more followed by change everything. In the end making cleaner code.

You should be able to use the below Plug line to use the dev branch for testing as we finish others currently active.

Plug 'n0v1c3/vira', { 'do': './install.sh', 'branch': 'dev'}
jamesl33 commented 4 years ago

@n0v1c3 Sound good, I haven't had a chance to look at your dev branch just yet but hopefully I'll get a chance this evening. I'll leave this PR open for you to close whenever you're ready.

n0v1c3 commented 4 years ago

@jamesl33 last push passes on the pull request thank you. I would still like to confirm that it will work with NO browser set in your vimrc. That will make it truly closed in my VIRA issues.

Thank you very much for making us aware of it and please keep letting us know all the other mistakes we make :+1: ! This keeps us testing if VIRA works for everyone.

jamesl33 commented 4 years ago

@n0v1c3 Great thank you for the update. Yes, this should work by default without having a browser set (that's how I've been using this by default). Of course this relies on having the 'open' command is installed; which should be by default on MacOS and most popular Linux based systems. In the event 'open' is not installed on the system, the user may provide the 'vira_browser' variable to use a specific command e.g. 'firefox'.

n0v1c3 commented 4 years ago

@jamesl33 I will most likely end up with a few more related pushes on this one to make sure the browser is working everywhere. I like and will need another push for the if open installed then go to my true Linux default. FYI order of operations should be if overridden with vira_browser, else if open exists, else Linux was right all along. I am sure more will come up in the future but I will add that part to my look into it. I will not check OS but if open

Although, the true goal is if you needed to go to the browser what didn't vira do, that pile is still there but we are working on it.

jamesl33 commented 4 years ago

@n0v1c3 I've created another pull request which I believe fulfills the requirements stated above. If you've got any comments/questions please let me know and I'll address them as soon as possible.