Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Width doesn't seem to adapt properly for multi-line items #68

Closed
parbo opened this issue Apr 3, 2023 · 14 comments
Closed

Width doesn't seem to adapt properly for multi-line items #68

parbo opened this issue Apr 3, 2023 · 14 comments

Comments

@parbo
Copy link

parbo commented Apr 3, 2023

If the first item shown is small it will display fine, but if then show a bigger item, and go back to the smaller item, then the width doesn't adjust down to the smaller size. Only the height changes.

However, if I make it show a single-line item, then the width goes back down.

@casouri
Copy link
Owner

casouri commented Apr 3, 2023

Just to clarify, you make eldoc-box show three different doc at three different locations, first and last small, the second one large, and for the third one the child frame doesn't automatically shrink? Could you show some screenshots? That'll be extra clear.

@parbo
Copy link
Author

parbo commented Apr 4, 2023 via email

@parbo
Copy link
Author

parbo commented Apr 4, 2023

Correct size:

image

Something bigger:

image

Back to the small one (now too wide):

image

Showing a single line doc and then the small one again, makes the small one have correct width again.

@casouri
Copy link
Owner

casouri commented Apr 27, 2023

Thanks. I think this is due to the markdown separator. You see, one of the prettifier replaces markdown's separator with a strike-through line with a display text property. The display property specifies that the width of the strike though is the width of the window. That might interfered with window-text-pixel-size, which return the size of the text in a window, and is used by eldoc-box to determine the frame size for the childframe.

Anyway, I added a (harmless) workaround that should fix this.

@mrsch
Copy link

mrsch commented May 24, 2023

This workaround breaks the childframe for me.
It causes the childframe to be 1 by 1 character in size sometimes:

1684941528_grim

@casouri
Copy link
Owner

casouri commented May 26, 2023

The workaround probably just exposed this problem, rather than caused it. Could you give me a reproduce recipe?

@deifactor
Copy link
Contributor

I've noticed the issue @mrsch mentioned happening when using the pure GTK version of emacs (I'm using sway; not sure about X or other compositors). it doesn't always happen, but it happens eventually if I just move point around enough. It goes away if I get rid of the workaround (but then of course I get the behavior the workaround fixes). I think something doesn't like setting the frame width rapidly and it gets 'stuck' on the first frame size.

@deifactor
Copy link
Contributor

Aha, it seems to consistently happen if and only if the new documentation frame would be the same size as the old one (moving point within an identifier, is one way to trigger this, but I verified it's not the only way). I think this is an emacs bug.

deifactor added a commit to deifactor/eldoc-box that referenced this issue Aug 10, 2023
There appears to be an Emacs bug where in some environments, calling
set-frame-size to another size and then to the original size will result in the
second call not taking effect. This means that the workaround for casouri#68 causes
the frame to shrink if moving between doc items with the same doc size (or
within the same doc item).

Instead, we just temporarily shrink the Markdown separators to make the window
size calculations work, then restore them afterwards.

* eldoc-box.el (eldoc-box--update-childframe-geometry): Adjust markdown
separator width instead of setting the frame to a tiny size.
@casouri
Copy link
Owner

casouri commented Aug 10, 2023

I added a workaround for the workaround. Not sure it'll fix the problem though, let's see :-)

@casouri
Copy link
Owner

casouri commented Aug 10, 2023

Oops, I just see your patch. Let me have a look!

@deifactor
Copy link
Contributor

deifactor commented Aug 10, 2023

Just tried and (redisplay t) doesn't fix it. The weird thing is that if I add

(message (format "%s %s" (frame-height frame) (frame-width frame)))
(run-with-idle-timer 0 nil (lambda () (message (format "%s %s" (frame-height frame) (frame-width frame)))))

then the first message has the size from (set-frame-size) and the second is 1 1.

@casouri
Copy link
Owner

casouri commented Aug 10, 2023

Ok, I pushed another fix following your idea. Could you give it a try?

@deifactor
Copy link
Contributor

It works! And that's a much cleaner solution.

@casouri
Copy link
Owner

casouri commented Aug 10, 2023

Great. Thanks for your PR, and the detailed commit message, I appreciate it 😊

@casouri casouri closed this as completed Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants