dbordak / telephone-line

A new implementation of Powerline for Emacs
GNU General Public License v3.0
550 stars 51 forks source link

telephone-line-minor-mode-segment removed from defaults #63

Closed bjorntropf closed 6 years ago

bjorntropf commented 6 years ago

Hi, in the last commit 220a1a4a89ca95be9ae1fab356b92296fff5b73a the telephone-line-minor-mode-segment was removed in line 310 of telephone-line.el.

It looks like this was removed by accident when adding other default segments.

Could you please add it back to the defaults? (So far this package was usable out-of-the-box.)

dbordak commented 6 years ago

That was intentional, actually. You can add it back in yourself if you want it, but the idea is to work on a whitelist rather than a blacklist -- most of the segments are intended to be better versions of minor mode lighters anyway.

dbordak commented 6 years ago

Just to expand on this, I didn't consider telephone-line to actually be usable out-of-the-box, diminish was practically required. There might be some lighters you're currently missing, but I do intend on adding more segments to compensate...

Alternatively, I could do a lightweight whitelist on the minor-mode-segment itself, in-package, since the list is already being parsed in there anyway. Would that be better?

bjorntropf commented 6 years ago

Thank you for your answer. I'll adjust my local configuration.

Regarding the general use case for this issue: I'm using delight (similar to diminish) for all minor modes except flycheck. Seeing the FlyC:5/18 (see screenshot here: https://github.com/flycheck/flycheck) in the mode line tends to be very useful. Maybe a future segment could compensate for flycheck and similar packages like flymake?

dbordak commented 6 years ago

Oh, yeah, a flycheck segment is planned (and in fact I had been working on it, I think I just forgot, hehe)

dbordak commented 6 years ago

@bjorntropf I've just added a flycheck segment to the defaults in the latest commit. It's a bit lame, though, since it has unconfigurable faces as of yet.

bjorntropf commented 6 years ago

@dbordak Nice, great work!

It's a bit lame, though, since it has unconfigurable faces as of yet.

This should be easily fixable for now by using the faces flycheck-warning and flycheck-error instead of orange/tomato.

Having the status text like ":)" or "Problems: " in a global alist or similar would make customisation a bit easier. But you might already have that planned ;-)

dbordak commented 6 years ago

Already tried that, those faces default to an underline, which is terrible for a mode-line :/ (and I can't rightly overwrite users' faces). Also tried the -list- variants but those somehow failed to propertize at all?

bjorntropf commented 6 years ago

One way to do it would be something like

(propertize "!" 'face `'(:foreground ,(cadr (member :color (face-underline-p 'flycheck-error)))))

but then you would have to make sure that the (cadr (member ...)) code is more robust than above.

The proper way would be to use two general alist variables and to leave the customisation to the user. So extracting both text parts from the function and providing some good defaults.

(('error . (propertize (format ...)) 'face ...)
 ('valid . (propertize ":)" 'face ...))
 ('running     "*")
 ('no-checker  "-")
 ('not-checked "=")
 ...)
dbordak commented 6 years ago

One way to do it would be something like [...]

Now we're depending upon users not overwriting the face -- it's just not really worth doing it. I could just add a couple new faces, though.

The proper way [...]

That's too much abstraction -- it'd be much easier to just have users define their own segment function. The alist is approaching the size of the function body anyway, and we're leaving out other customization people would want like hover style.