search

corgi-emacs / corgi

Unbundled Emacs configuration aimed at Clojure developers
180 stars 18 forks source link

corgi-stateline not working #15

Closed theophilusx closed 2 years ago

theophilusx commented 2 years ago

Fresh install of corgi under Emacs 28.1

Only just noticed there was a corgi-straightline package. This was mainly because it doesn't seem to be working. Had a quick look at the sources. Seems that it is operating on a face called mode-line-active, but there is no such face. There is mode-line and mode-line-inactive, but no mode-line-active.

Currently configuring my own fork of corgi so that I can provide some pull requests and will see if I can fix this.

plexus commented 2 years ago

You're right that it's not working at the moment... it definitely worked in the past, not sure what changed.

I do have a mode-line-active face:

Face: mode-line-active (sample) (customize this face)

Documentation:
Face for the selected mode line.
This inherits from the ‘mode-line’ face.

Defined in ‘faces.el’.

           Family: unspecified
          Foundry: unspecified
            Width: unspecified
           Height: unspecified
           Weight: unspecified
            Slant: unspecified
       Foreground: unspecified
DistantForeground: unspecified
       Background: unspecified
        Underline: unspecified
         Overline: unspecified
   Strike-through: unspecified
              Box: unspecified
          Inverse: unspecified
          Stipple: unspecified
             Font: unspecified
          Fontset: unspecified
           Extend: unspecified
          Inherit: mode-line

  This face was introduced, or its default value was changed, in
  version 29.1 of Emacs.
plexus commented 2 years ago

Correction, it actually seems to work fine for me... But seems that font-face is pretty new (Emacs 29.1)? Maybe there's a better way that's a bit more backwards compatible.

theophilusx commented 2 years ago

Ah, so looks like mode-line-active facce is being introduced in Emacs 29, the current development version. Definitely isn't in Emacs 28.1 (current stable version) and no mention of it in any of the NEWS files for emacs.

I changed mode-line-active to just mode-line and it appears to then work (though default colors need a bit of tweaking to work well with the default theme corgi loads). However, I'm a little concerned about the fact it just adds new mappings and never removes them. I get the impression from the documentation that each call to face-remap-add-relative adds another mapping rather than replacing previous mappings. The call does return a cookie which can be used to remove the mapping, so I'm testing a change where we keep that cookie and remove the mapping before adding anew mapping. It appears to be working.

I also thought it would probably be good if we could make this support being toggled on/off. , so playing wiht that to see how easily this can be done reliably.

On Mon, 18 Apr 2022 at 16:50, Arne Brasseur @.***> wrote:

You're right that it's not working at the moment... it definitely worked in the past, not sure what changed.

I do have a mode-line-active face:

Face: mode-line-active (sample) (customize this face)

Documentation:

Face for the selected mode line.

This inherits from the ‘mode-line’ face.

Defined in ‘faces.el’.

       Family: unspecified

      Foundry: unspecified

        Width: unspecified

       Height: unspecified

       Weight: unspecified

        Slant: unspecified

   Foreground: unspecified

DistantForeground: unspecified

   Background: unspecified

    Underline: unspecified

     Overline: unspecified

Strike-through: unspecified

          Box: unspecified

      Inverse: unspecified

      Stipple: unspecified

         Font: unspecified

      Fontset: unspecified

       Extend: unspecified

      Inherit: mode-line

This face was introduced, or its default value was changed, in

version 29.1 of Emacs.

— Reply to this email directly, view it on GitHub https://github.com/lambdaisland/corgi/issues/15#issuecomment-1101152724, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMFIKOGAPMM5HX6X2Y7TODVFUA4NANCNFSM5TU5DH6A . You are receiving this because you authored the thread.Message ID: @.***>

-- regards,

Tim

-- Tim Cross

plexus commented 2 years ago

Thanks! That all sounds good.

theophilusx commented 2 years ago

Have submitted a PR which makes some fairly significant changes - main one being that it turns corgi-stateline into a minor mode. Iff you decide to merge it, I will update the manual. If we want the mode enabled by default, it will also be necessary to update the sample config package statement to call corgi-stateline-mode to turn it on.

theophilusx commented 2 years ago

@plexus I have run into an issue with the corgi-stateline-mode implementation. Basically, disabling the mode requires a much more sophisticated tracking of face mappings. Example scenario which demonstrates the issue

In the new buffer, the modeline reverts to the colors it had before (i.e. default modeline colors).

At this point re-enabling and disabling corgi-stateline-mode doesn't help. The remapping cookie has been lost and now there is no way to remove the old mapping and restore to defaults.

I will need to dig into the docs to see if there is some way of globally removing all face remapping or (more likely) will need to create a mode variable which tracks the remapping cookie for each buffer and then when disabling the mode, iterate through and remove for each buffer.

I'm also considering changing how the normal state colors are determined. I think things would work better if we define the default bg and fg colors for normal state from the default mode line colors i.e. when in normal state, the mode line will be the same regardless of whether the mode is enable or disabled. It will only change color when you change into insert, visual or emacs state.

Finally, one other niggle with define-minor-mode. When you do M-x corgi-stateline-mode, it echos the message "You can run the command 'corgi-stateline-mode' with M-x corg-m RET'. I have no idea where/how this message gets created. There is nothing in the docs about it and it does not seem to appear when you macroexpand define-minor-mode. May need to ask on the emacs devel list about this one. I suspect it is Emacs trying to be too helpful.

plexus commented 2 years ago

I think this is all fairly easily solvable. I took the liberty to poke at it a bit, see https://github.com/lambdaisland/corgi-packages/commit/a43af6d3edb4d75beda8a9be8205610290f38cdb , this seems to be working fine for me so far. Maybe try cherry picking that into your branch?

I've used globalized-minor-mode, that's a pattern I'm more familiar with, and it means people can still use the minor mode more selectively on certain buffers if that's what they prefer.

I also did what you suggested, revert to the default face in normal-mode. I guess we should get rid of the defcustoms for normal-mode as well.

plexus commented 2 years ago

celebrate