Eli Zaretskii writes: >> From: Manuel Giraud >> Cc: rudalics@gmx.at, 73734@debbugs.gnu.org >> Date: Sat, 12 Oct 2024 16:10:10 +0200 >> >> Eli Zaretskii writes: >> >> [...] >> >> > This doesn't align bindings if a variable-pitch font is used, but >> > it still produces a better display than the current code does when >> > variable-pitch font is the default font. >> >> Thanks, yes this is better to remove the confusion (but sometimes misses >> bindings alignment). To finalize this patch, I still have two questions >> left: >> >> - What is the purpose of the "(min 30)"? Because if the window >> is large enough this will always return 30. > > First, I didn't write this code and don't consider myself an expert, > so what follows are basically my guesses. (In the code I proposed I > simply didn't touch those parts to clearly separate my changes from > the rest.) > > I think the minimum of 30 is there to make sure we don't produce just > 2 sets of columns when the window is wide enough to allow more. This > uses the screen real estate more efficiently, so the *Completions* > window is not as tall as it would be with just 2 sets of columns. Ok, that makes sense. I was under the impression that 30 is too small but you're right that it could (should?) stay the same. >> - Should we use frame-width instead of window-width? It seems >> that when this function is called (get-buffer-window >> "*Completions*") always returns nil. > > I think we should fall back to frame-width, but use the width of the > *Completions* window if it exists. I'm quite sure there's a sequence > of commands which will leave the *Completions* buffer on display, and > in that case the window showing it will be reused. Alright, so what about this new version of the patch? FWIW, I have tested it lightly and the only time the keybinding alignment is off is when menu entry is quite long and we needed the 2 spaces separation with the keybinding.