oantolin / embark

Emacs Mini-Buffer Actions Rooted in Keymaps
GNU General Public License v3.0
938 stars 56 forks source link

Zebra mode overlay takes precendence over candidate background #164

Closed minad closed 1 year ago

minad commented 3 years ago

In consult-mark I am using a background face to visualize the point position. The background is overwritten by the zebra overlay since overlays win over text properties. Any ideas how we can work around this? consult-register-window has the same problem, since I am also using overlays there.

minad commented 3 years ago

What about using thin lines? I think I had proposed that before, but I don't recall if there was a reason to prefer the zebra.

(setq-local inhibit-point-motion-hooks nil)
(insert (concat
         "line1"
         (propertize (concat "\n" (propertize " " 'display '(space :align-to right)) "\n")
         'face '(:height 0.01 :underline t :foreground "#ccc")
         'cursor-intangible t 'intangible t)
         "line2"
         (propertize (concat "\n" (propertize " " 'display '(space :align-to right)) "\n")
         'face '(:height 0.01 :underline t :foreground "#ccc")
         'cursor-intangible t 'intangible t)
         "line3"))
oantolin commented 3 years ago

I guess one reason is that the "thin" lines still take up a normal height line in the terminal version of Emacs.

oantolin commented 3 years ago

Also, I think it was I that proposed the lines, for consult-yank, and you vetoed it. 😉

minad commented 3 years ago

And I thought I voted for lines. The snippet above creates lines of height zero by scaling the face. This works only in graphical mode obviously. I thought about removing zebra mode from the consult register view and replace it by that. Or offer it as an alternative to zebra. But since the zebra hides the marks it is really not good for registers.

oantolin commented 3 years ago

Another option is to reimplement zebra stripes with text properties.

oantolin commented 3 years ago

My old completing-read yank function that I used before Consult added lines to the candidates.

minad commented 3 years ago

Do you prefer lines or zebras? I think about removing zebras in favor of lines. Alternatively provide both variants as options.

oantolin commented 3 years ago

Well, it does bother me a bit that lines take up an entire row in emacs -nw. (Well, I'm assuming, I haven't tested, is that really what happens?) But that's me just looking out for terminal Emacs users, I personally never use emacs -nw.

That's the only thing, except for that, I don't have any reason to prefer lines over zebra stripes or viceversa.

oantolin commented 3 years ago

I found our previous discussion of lines and stripes: minad/consult#44. If I had to guess you mostly vetoed it because the line was put on the individual candidates in the minibuffer. But lines in a register preview buffer or in an embark collect buffer are better, right?

minad commented 3 years ago

I replaced the zebras with lines now for the register view. Please give it a try! Shall we replace it for Embark snapshot too?

oantolin commented 3 years ago

Oh, you just don't separate the registers at all in the terminal? It might be worth it to waste a line on dashes or something, right?

oantolin commented 3 years ago

You know, the register preview doesn't really need either lines or stripes, since you can deduce where each register ends from indentation.

minad commented 3 years ago

Yes I know, I still like it more with the lines. If you have multiple strings with indentation it looks very messy.

oantolin commented 3 years ago

The lines do look nice! I'll try them out for embark.

minad commented 3 years ago

Already done, see my draft PR #167. I am a proponent of not using emacs on the terminal so for now I decided to not add any separators there. I would also not like to waste full lines. And I don't want to have both zebra and line options for the register view.

oantolin commented 3 years ago

I also don't want both zebra stripes and lines for embark collect. I took a look at your PR, but think I have a simpler implementation strategy (which I actually could have used for the zebra stripes too, I just didn't think of it back then): just look for new lines outside of buttons! Also, I don't want to leave terminal Emacs users out in the cold: since the lines are always explicitly requested, either by toggling or by configuring embark-collect-initial-view-alist, I think they should do something in the terminal too.

minad commented 3 years ago

Sounds good! I close my draft PR then.

oantolin commented 3 years ago

I just checked and browse-kill-ring by default dedicates a whole to separate the kill ring entries. It looks fine. I won't waste a whole line in graphical Emacs but I won't feel guilty about wasting a line in terminal Emacs either.

minad commented 3 years ago

Yes, I know. Browse-kill-ring always wastes a whole line. I don't like this, but given that it is opt-in it is all good.

oantolin commented 3 years ago

And I'll only waste a line in terminal Emacs, so it won't even affect us. 😉

oantolin commented 3 years ago

I got obsessed with finding a single implementation for graphical and terminal Emacs that doesn't waste a line. I came up with this: underline the last line of each candidate, and also add an overlay on the newline character, with a before-string consisting of an underlined (space :align-to (- right 1)) (if you go all the way to right terminal Emacs puts a $ in the margin).

It works,, it doesn't waste a line... but I don't like how it looks! 😆 The underline is too close to the text. It doesn't look like a separator, it looks more like... like the last line is underlined (of course). Plus you can't control the color of the underline: each portion of it is the same color as the foreground color of the corresponding text.

Terminal: image

Graphical: image

I'll think I might scrap that and go back to wasting a line, except of course, that the line will have a height of 1 pixel in graphical Emacs.

oantolin commented 3 years ago

Well, now that I'm looking at them again, the spacing in graphical Emacs is not so bad, it does look a little separatorish, not so underliney.

oantolin commented 3 years ago

Haha, back in December I said, "One thing one could do is look for the last line and display it underlined." But obviously I didn't actually try it then.

minad commented 3 years ago

Hmm I like it more to use a separate line for the separators. I would really like to control the color of the line to make it somehow dim. This you can unfortunately not achieve by doing the underlining. For this reason I did not try this approach. On terminal wasting a line or using the underlining is both fine. But I don't have hope for a single implementation. As you have it now the quality of the graphical view is worse due to the legacy support.

oantolin commented 3 years ago

I'll go with the separate line, even if that takes up a whole line in the terminal. I didn't do it yesterday because I wasted my allotted Emacs time on trying different things to do with underlining...

minad commented 2 years ago

@oantolin Is there still interest in this? Do you want me to revive my https://github.com/oantolin/embark/pull/167? Also it would be nice to fix #389 at some point.

oantolin commented 1 year ago

Finally got around to adding horizontal line separators. I decided to not make it configurable: if any candidate within a group has a newline, then we add separators between candidates for that group. 02962faa43f25c6de9b053271963ce27d4c4fba5

minad commented 1 year ago

You could maybe make it configurable by handling embark--hline=nil.

oantolin commented 1 year ago

Sure, if the need arises, but I can't imagine anyone would prefer to have multiline candidates all run together.