zefei / vim-wintabs

Modern buffer manager for Vim
MIT License
325 stars 25 forks source link

Improvements #2

Closed cosminadrianpopescu closed 6 years ago

cosminadrianpopescu commented 9 years ago

I've added a few improvements to wintabs and also corrected a few bugs:

New options: wipeout_buffers_oncloce

If true, when closing a buffer, this will also be wiped out. The default is 0, so the default behavior is not changes.

New function: wintabs#previous

This is changing to the last accessed tab. So, this way, you can easilly change between the last 2 edited tabs.

Numbers in buffer names.

This way we can very easilly call WintabsGo.

Bug fix: Saving the tabslist to session

Since, according to the VIM documentation, only string and number variables are saved using the globals in the sessionoptions, the tabs were not restored after a session restore. I've fixed that by adding a new variable, g:Str_wintabs_session, which holds the string serialization of g:Wintabs_session. When the session is restores, the g:Wintabs_session variable is restores from the string variable.

Bug fix: Restoring the session might on empty windows

Sometimes, an empty window gets saved in the g:Wintabs_session variable. When we encounter such a variable on restore, we just ignore it. If it's in the second window, will break the session restore, since the window is not restored, but the w:wintabs_buflist variable is cleared.

zefei commented 9 years ago

Hello, thank you for the PR. However this PR contains separate issues that are better put into separate PRs. Below is my comment on each issue:

New options: wipeout_buffers_oncloce

I want to do this, but this is more involved than that single function. If user sets this option, I'd expect it also functions when closing windows or tab pages or using *Only functions. It's also needed to consider the case when closing buffer fails.

New function: wintabs#previous

I think :e # is a better solution here.

Numbers in buffer names

Originally I had this under an option, but I removed it because no modern tabbed interface shows tab numbers, even if they have numbered switch shortcuts (like ctrl/cmd-n in Firefox and Chrome). WintabsGo is provided as a way to map such functions to numbered key combos such as <C-1>, <C-2>, etc. They typically don't need visual cues to work well.

Bug fix: Saving the tabslist to session

Nice. I may need to further test this against xolox/vim-session though as I want to guarantee compatibility with that plugin.

Bug fix: Restoring the session might on empty windows

Can you give me more details on the bug? I can't reproduce it.

cosminadrianpopescu commented 9 years ago

New options: wipeout_buffers_oncloce

I want to do this, but this is more involved than that single function. If user sets this option, I'd expect it also functions when closing windows or tab pages or using *Only functions. It's also needed to consider the case when closing buffer fails.

I will also look into this. I only use tabs, so this is why I didn't check these options, but you are right.

New function: wintabs#previous

I think :e # is a better solution here.

If you only do :e #, then

execute a:confirm ? 'silent! confirm enew' : 'enew!'

or

let buffer = w:wintabs_buflist[a:n]
execute a:confirm ? 'silent! confirm buffer '.buffer : 'buffer! '.buffer

from the s:switch_tab function will not get executed. This is why I implemented this function. So, the switching is done properly only using wintabs functions.

Bug fix: Saving the tabslist to session Nice. I may need to further test this against xolox/vim-session though as I want to guarantee compatibility with that plugin.

I am not using xolox/vim-session, I am using sessionman plugin. But I suppose that even xolox/vim-session should do a mksession and so at some point. Even if this is not the case, my fix should not break the xolox plugin, since it does at the end of the day restores the session from a string instead of a dictionary variable. So it should work also with xolox. But let me know if it doesn't. The problem with the xolox plugin (which I analized) is that is using the xolox misc plugin which includes some kind of multi threading simultation which doesn't sound very VIM like. I don't event think that it will work that well on cygwin (at work I am using VIM under cygwin).

Bug fix: Restoring the session might on empty windows

Can you give me more details on the bug? I can't reproduce it.

I can't reproduce it myself. But, take this case for example:

g:Wintabs = {'1': {'1': ['/tmp/f1', '/tmp/f2'], '2': []}, '2': {'1': ['/tmp/f3', '/tmp/f4']}}

The thing is that at some point, in the session, I've had a window which became void of tabs (see window number 2 from tab number 1). Without this fix, when you restore a session, the w:wintabs_buflist variable for tab number 1 will be empty, so you will only see the buffer '/tmp/f1' in the status line. This is because the second window is not displayed. The bug is somewhere when closing the windows (the '2': [] list should not exist in the dictionary because the window is closed). However, it was easier to fix it when the sessions is loading. If the window is empty, I suppose anyway we can discard it.

I think that the empty window remains there when I have some windows opened with buffers which are not text buffers (like help buffers or something like this), but I am not sure.

Numbers in buffer names

Originally I had this under an option, but I removed it because no modern tabbed interface shows tab numbers, even if they have numbered switch shortcuts (like ctrl/cmd-n in Firefox and Chrome). WintabsGo is provided as a way to map such functions to numbered key combos such as , ,

Ok. For me it is very important, because I can very easily call WintabsGo n when I have more than 15 buffers open at the same time (which is quite often, since I am using VIM at my work place).

cosminadrianpopescu commented 9 years ago

One more remakr about this:

"I think :e # is a better solution here."

Also, doing :e # will fail if the buffer that we are currently in is not saved, which is kind of annoying.

On Wed, Dec 10, 2014 at 12:06 AM, Zefei Xuan notifications@github.com wrote:

Hello, thank you for the PR. However this PR contains separate issues that are better put into separate PRs. Below is my comment on each issue:

New options: wipeout_buffers_oncloce

I want to do this, but this is more involved than that single function. If user sets this option, I'd expect it also functions when closing windows or tab pages or using *Only functions. It's also needed to consider the case when closing buffer fails.

New function: wintabs#previous

I think :e # is a better solution here.

Numbers in buffer names

Originally I had this under an option, but I removed it because no modern tabbed interface shows tab numbers, even if they have numbered switch shortcuts (like ctrl/cmd-n in Firefox and Chrome). WintabsGo is provided as a way to map such functions to numbered key combos such as , , etc. They typically don't need visual cues to work well.

Bug fix: Saving the tabslist to session

Nice. I may need to further test this against xolox/vim-session though as I want to guarantee compatibility with that plugin.

Bug fix: Restoring the session might on empty windows

Can you give me more details on the bug? I can't reproduce it.

— Reply to this email directly or view it on GitHub https://github.com/zefei/vim-wintabs/pull/2#issuecomment-66375906.

zefei commented 9 years ago

Also, doing :e # will fail if the buffer that we are currently in is not saved, which is kind of annoying.

Oh, in that case you are not using :set hidden, then you can use either :confirm e # or :e! #. These have exactly the same effects as the switch_tab function. I have :set hidden, so for me :e # is just :e! #. This has the side benefit of re-opening a mis-closed buffer.

On tab numbers, I think one way of doing it would be adding an option to show either tab number or buffer number or nothing, since I know there are many people quite used to :b(!) n. However, in practice, if you can see the buffer name in the list, using <Plug>(wintabs_next) repeatedly is probably faster. But anyways, having this behind an option is a good compromise I think.

I don't have time this week to fully test the bug fixes. Maybe I'll test the PR next week,

cosminadrianpopescu commented 9 years ago

I've found another problem with the :e # solution: If the buffer is in another tab, then it is brought in the current tab, which I don't think it's ok. While the wintabs#previous function will just not change the buffer. In my opinion this is the desired behavior, since wintabs is trying to scope the buffers to the tabs.

zefei commented 9 years ago

You are right, in case user just switched window/tab, :e # indeed isn't the right way.

cosminadrianpopescu commented 9 years ago

Anyways, just as a principle, if I use wintabs plugin, I prefer do to all buffers changing with wintabs.

zefei commented 6 years ago

All the fixes in this PR have been implemented in some fashion. Closing this PR.