search

alphapapa / activities.el

Activities for Emacs (suspend and resume activities, i.e. frames/tabs and their windows, buffers)
GNU General Public License v3.0
202 stars 12 forks source link

Need to display tabs correctly with tab-bar-format-tabs-groups under tab-bar-mode #107

Open shipmints opened 3 months ago

shipmints commented 3 months ago

activities needs to respect tab-bar-mode's tab-bar-tab-group-face-function override in the same way it currently respects tab-bar-tab-face-function. The lack of support means that the activities-tab face is never applied.

A PR coming shortly with the tested fix for this.

Configuration snippet for testing:

  (setq tab-bar-format
        '(
          tab-bar-format-tabs-groups ;; this shows groups
          ;; tab-bar-format-tabs ;; switch to this for plain tabs
          tab-bar-separator
          activate
          tab-bar-format-align-right
          tab-bar-format-global
          ))

With groups enabled, you'll have to assign a group to one tab to get the group to appear and for the broken formatting path to be invoked.

You can invoke this to ensure there's at least one group for testing (tab-bar-change-tab-group "*default*")

or call M-x tab-group to assign a name interactively.

P.S. While reading through the tab-bar-mode code, it appears there is a bug that, if fixed, might render the PR correction unneeded. Perhaps in Emacs 30 if this is truly a bug and gets reported.

(defun tab-bar-tab-group-face-default (tab)
  (if (not (or (eq (car tab) 'current-tab)
               (funcall tab-bar-tab-group-function tab)))
      'tab-bar-tab-ungrouped
    (tab-bar-tab-face-default tab)))

At the end of the function, tab-bar-tab-face-default is called and not tab-bar-tab-face-function so the override is never applied when groups are being formatted. Since the override would have been set to activities-tabs--tab-bar-tab-face-function the custom face is never applied. Also, to get around this, one can't mix both the tab format and the group format or the tabs get repeated.

shipmints commented 3 months ago

See #108.

alphapapa commented 3 months ago

P.S. While reading through the tab-bar-mode code, it appears there is a bug that, if fixed, might render the PR correction unneeded. Perhaps in Emacs 30 if this is truly a bug and gets reported.

Would you please report that bug, then? If it is truly a bug (you should probably cc Juri on it, as he's the maintainer of that library), then it might not be too late to get it fixed in Emacs 30. Then, if it's as you say, we might be able to omit the change here.

shipmints commented 3 months ago

Bug reported for confirmation https://lists.gnu.org/archive/html/bug-gnu-emacs/2024-06/msg01468.html

shipmints commented 3 months ago

Looks like we need this small change afterall. I did not check Juri's contention. It's his code.

From: | Juri Linkov Subject: | bug#71733: 29.3; tab-bar-tab-group-face-default invokes tab-bar-tab-face-default incorrectly? Date: | Sun, 23 Jun 2024 20:30:48 +0300

tags 71733 wontfix close 71733 30.0.50 thanks

tab-bar-tab-group-face-default' hard codes the invocation to tab-bar-tab-face-default' but should it instead invoke `tab-bar-tab-face-function' to benefit from a custom function?

(defun tab-bar-tab-group-face-default (tab) (if (not (or (eq (car tab) 'current-tab) (funcall tab-bar-tab-group-function tab))) 'tab-bar-tab-ungrouped (tab-bar-tab-face-default tab))) ;; should the last line be something like this? (funcall tab-bar-tab-face-function tab)))

I checked the latest `tab-bar' source code this morning and this hasn't yet been addressed, if indeed this is unintended behavior.

Sorry, this can't be changed because it will cause an infinite recursion. You can see in 'tab-bar-format-tabs-groups':

               (let ((tab-bar-tab-face-function
                      tab-bar-tab-group-face-function))
                 (tab-bar--format-tab tab i))

Then with the default value 'tab-bar-tab-group-face-default' of 'tab-bar-tab-group-face-function', 'tab-bar-tab-face-function' in 'tab-bar-tab-group-face-default' will recursively call itself.

alphapapa commented 3 months ago

Ok, thanks for checking on that.

shipmints commented 3 months ago

Juri said in a later email to me that he will improve the documentation. Our approach is the right one.

shipmints commented 3 months ago

Fast. https://github.com/emacs-mirror/emacs/commit/4a0958642d96fcd521087dc4fa54d0701797a585