Thanks for the PR, we were considering doing something in this direction so this helps a lot! There is one main thing missing. Some people don't want the pinned tab button on the top row but still want to have the close button. This is currently not possible with this change. I suggest when the user picks workbench.editor.pinnedTabButton: off to fall back to a close button in that case. I made some small fixes and changed the setting name to align with the naming of tabCloseButton

I suggest to change this a bit. I don't see the beneefit of having close action right and unpin action left. Introducing this will also break the current users settings when updating. I suggest to have the following settings:

• tabActionLocation: ['left', 'right']
• tabActionCloseVisibility: boolean
• tabActionUnpinVisibility: boolean

tabActionLocation defines the location of the actions on the tabs. If a tab is pinned and tabActionUnpinVisibility is set to false, we show the close action as long as tabActionCloseVisibility is set to true, otherwise we don't show it. This will also allow the migration of the settings to be easier and not break the users logic.

What do you think @n-gist ? Are you up to makeing the change? I can give you some pointers if you need help, otherwise I can make the change if you wish.

@benibenj

However, removing the unpin action when the user only has one tab bar can become very confusing as there is no more indication that a tab is pinned.

Was thinking about that too, but since default options are 'right', I thought that the one who sets it to 'off' undestands that he hides pin icon even on combined bar. Also was thinking about making pin button 'off' not working on combined row by default, and adding a separate setting like 'hide pin icon if its option is set to off, even when pinned tabs are on one row with regular tabs'. But this would add disambiguity to tabUnpinButton option.

There is one main thing missing. Some people don't want the pinned tab button on the top row but still want to have the close button.

I have tested your change, I like how it works. Solves my issue, and adds even more flexibility.

I suggest to change this a bit. I don't see the beneefit of having close action right and unpin action left.

True, the number of people willing to use it is most likely too small.

I suggest to have the following settings:

• tabActionLocation: ['left', 'right']
• tabActionCloseVisibility: boolean
• tabActionUnpinVisibility: boolean

Something like that I thought when saw the code to edit. There should be general functionality over both actions. But one of questions about that is, should the tab action be single? Will there be other actions for tabs in the future, where more than one should be displayed and controlled separately?

Yeah I can try to implement this.
I just checked some code about that, and I think we can make intermediate class between CloseOneEditorAction/UnpinEditorAction casses and Action class for tab actions, to add a visibility property inside it. Is it worth doing? Any other action classes should inherit form it also?

Made the changes. Second commit (Mount...) is about generalizing things for tab actions by making TabAction class for it. May need renaming etc. Maybe it is possible to get options object without passing it to Visibility method? I'm not familiar with the code to figure it out on my own. Can be reverted if not needed.

Another thing I thought is - what if someone wants to have close buttons on regular tabs, but no close buttons on pinned ones? Having no close button on them kinda protects them a bit from accident closing by missclicks. This may still force us to add another setting for that if there is no better ideas.

There is an idea to avoid adding a separate setting for this - make the visibility settings as follows, instead of boolean ones:
tabActionCloseVisibility = 'always' | 'unpinned' | 'pinned' | 'off'
tabActionUnpinVisibility = 'always' | 'unpinned' | 'pinned' | 'off'

To be more correct.

1. There is no 'unpinned' state for unpin action, but it have a difference whether the pinned tabs are on separate row.
2. I think intermediate class is now important, because if we go this way, the visibility code will be a little more complex. Good to have it not in the redrawTab function, but in a separate place.
   tabActionCloseVisibility = 'always' | 'unpinned' | 'pinned' | 'off'
tabActionUnpinVisibility = 'always' | 'normalRow' | 'separateRow' | 'off'
3. I also was thinking about that close button have additional state, when it is pinned AND on separate row. But still, it seems to me that the user wants to control its visibility only based on the pinned status, it makes no difference whether the row is separated for close button. This would also require making the visibility settings as flags rather than strings. I don't think this is necessary.

I did a bit of refactoring and added settings migration which makes sure that users which used the old setting get the correct new settings. Otherwise it looks great. I think the way it works now is simple enough for everyone to understand and solves all the problems that users want to fix. We have one month before this will be released to stable, giving us enough time to see what people think about it.