centaur-tabs icon indicating copy to clipboard operation
centaur-tabs copied to clipboard

Implement auto hide tabs after delay feature

Open dalanicolai opened this issue 3 years ago • 6 comments

Finally I made some time to try to implement issue #129. I am not sure about the cleanest way how to implement this. For now I it is necessary to call centaur-select-tabs-forward instead of centaur-tabs-forward because that function selects which function to select for switching tabs (depending on the value of the centaur-tabs-auto-hide variable).

Anyway, this PR does not break anything, just if you want to use the auto hide feature you should use centaur-select-tabs-forward instead of centaur-tabs-forward to make it work.

dalanicolai avatar Dec 17 '20 10:12 dalanicolai

Just checking. You think you have time to look at this/merge it soon? No problem, otherwise. But then we could at least already add it to the Spacemacs layer. I know it is not the most beautiful implementation, but it's a start.

It is not an essential feature too of course, and Centaur tabs users who like it can find the code here. For Spacemacs, I can change the layer whenever this gets merged here.

dalanicolai avatar Dec 28 '20 20:12 dalanicolai

@dalanicolai hi! Sorry I've hadn't had the time to properly deal with centaur-tabs maintenance recently given I am currently obtaining a master's degree which consumes most of my time. I will probably try to find a collaborator soon that can help me with issues and reviews given that centaur-tabs is usually pretty delicate and some changes may unexpectedly cause performance problems or other issues so modifications have to be done very carefully. I really apologize for the inconvenience! centaur-tabs is my Emacs pride and I'll try to give it the attention and maintenance it deserves or in its defect, I'll try to look for somebody to help me with the task.

ema2159 avatar Feb 26 '21 22:02 ema2159

@ema2159 Hi! Thanks for your anwser and it is no problem at all, as it is not a very important feature of course. And the feature has in the meantime already been implemented into Spacemacs. Also, I don't even use tabs myself (I tried them out and then wanted this feature, but ultimately I think I do not need tabs as I came up with a different tab switching mechanism for now that I described here, of course it is simple but also I think it is rather nice). Anyway, I still think the feature could be nice for tabs users. I have tested the Spacemacs version for a while, and it seems to work perfectly fine. But indeed to nicely implement into your package itself was a little trickier and I have not thoroughly tested it.

Of course very good that finishing your master gets your priority and I wish you good luck and a lot of fun with it. I hope after that you will have time again to share nice improvements with the Emacs community :)

dalanicolai avatar Feb 28 '21 15:02 dalanicolai

@dalanicolai Hi! I'm going to be co-maintaining this project with @ema2159. This is a great feature to have, thanks for your contribution.

The PR looks great, except for one place you could've used pcase instead of a long bunch of conds to be more idiomatic:

(cond ((equal arg 'backward)
         (centaur-tabs-backward))
        ((equal arg 'forward)
         (centaur-tabs-forward))
        ((equal arg 'backward-group)
         (centaur-tabs-backward-group))
        ((equal arg 'forward-group)
         (centaur-tabs-forward-group)))

Is there a good reason to not use pcase here?

nebhrajani-a avatar Apr 20 '21 11:04 nebhrajani-a

@nebhrajani-a Great that you will pick this up! There was no good reason for this, just I did not know about pcase yet back then. Thanks for the instructive feedback.

dalanicolai avatar Apr 20 '21 16:04 dalanicolai

@dalanicolai This looks ready to merge --- however, would you like to update the README to show users how to use the hide feature? Please do mention that the switching function is 'centaur-tabs-select--' to use hiding correctly, and please don't forget to add your name for credit, in the format "Thanks to dalanicolai".

nebhrajani-a avatar May 02 '21 08:05 nebhrajani-a