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

Remove Bclose #61

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Remove Bclose #61

wants to merge 4 commits into from

Conversation

francoiscabrol
Copy link
Owner

@enthudave opened the PR #60 to fix an issue with the alternate buffer that was not well define after closing ranger.
I was trying to find an alternative implementation when I have seen a way to re-implement the neovim "rangerCallback" function without the use of the external dependency Bclose.
I wanted to remove Bclose since the beginning but I didn't find any other implementation keeping the same behaviours until now.

I know that there is probably some broken features so I open the PR to see if I get some interesting comments. During this time I will polish the implementation and make the thing totally stable.

@francoiscabrol francoiscabrol mentioned this pull request May 16, 2018
@enthudave
Copy link
Contributor

mmm, one small issue I spotted, and testing confirmed,
scenario:
open vim, open a file, split window, open another file, call any of the 'new tab edit' commands,
choose a file (or even when you don't choose one I think)...

what happens:
the file opens in the new tab but in the previous tab, the (split)window that ranger used is closed.
This is not intended to happen, right?

P.S. : on a different note, on line 54 the word 'regularly' does not mean what you think in this context,
what you are actually saying here is what you would say with something like 'de temps en temps' in french ;-),
'normally' would be a better choice. I don't mean to be an ass by correcting you on that, we all learn a little every day right?

@enthudave
Copy link
Contributor

enthudave commented May 16, 2018

might i suggest a wild idea?
It's been discussed in PR #4 ,
just open ranger in a new tab, both problems we are currently trying to solve would disappear,
we wouldn't need :Bclose! and the alternate buffer would not be changed since this is local to windows.
I also see one other major advantage with that: one of the nice things about ranger is the preview, and the column navigation, using a new tab for showing ranger would mean the user gets to enjoy maximum screen space. It could of course be added as an option if you don't really like it.
But in any case, I've tried this last week, and only needed to add two words to implement this,
:tabnew instead of :enew, before the termopen calls, and :bdelete! as the first line in the callback.

@enthudave
Copy link
Contributor

I think that line 170 with the s:std_in variable can be removed as it was in master

@enthudave
Copy link
Contributor

I've added a 'tab-feature' branch on my fork, so you can take a look at what I've got

@francoiscabrol
Copy link
Owner Author

Thanks for your help and sorry for the delayed response.
You addressed several good points:

  • about the the tab-feature I would like to add this behaviour as an option. I agree with you it solves the previous problem and simplify the implementation but I don't want to change this by default. The reason is that the current users are not expecting a change in the main feature behaviour.
  • about the issue that you had with the tab that close after closing ranger if the previous buffer is empty: I have seen that but I was wondering if it was an issue or not. I know that the correct behaviour would be to keep the tab open but I didn't find an easy fix (yet).
  • about this pull request, removing Bclose will be a good thing but I will not merge this alternate implementation except if I am sure that it is more stable than the current one. The current implementation is not perfect but at least it works well except some minor issues.

@enthudave
Copy link
Contributor

enthudave commented May 31, 2018

I've been taking another look at this.
Im still wondering if keepalt shouldn't be able to solve this.
Anyway why I reply now is to let you know about an oversight,
I hadn't noticed this before, until doing some testing (with master branch): the buffer that was open in the window that ranger uses gets deleted. Thinking about why this happens I've noticed there's no enew before the termopen() calls, I think this is needed as the help files for termopen() states:
'Spawns {cmd} in a new pseudo-terminal session connected to the current buffer.'

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

Successfully merging this pull request may close these issues.

2 participants