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

Implement auto hide tabs after delay feature #144

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

dalanicolai
Copy link

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
Copy link
Author

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.

@ema2159
Copy link
Owner

ema2159 commented Feb 26, 2021

@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.

@dalanicolai
Copy link
Author

dalanicolai commented Feb 28, 2021

@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 :)

@nebhrajani-a
Copy link
Contributor

nebhrajani-a commented Apr 20, 2021

@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?

@dalanicolai
Copy link
Author

dalanicolai commented Apr 20, 2021

@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.

@nebhrajani-a
Copy link
Contributor

@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".

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.

None yet

3 participants