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

Fix recursion in debug advice #184

Closed
wants to merge 1 commit into from
Closed

Fix recursion in debug advice #184

wants to merge 1 commit into from

Conversation

bmag
Copy link
Owner

@bmag bmag commented Apr 2, 2021

As reported in #182 (comment), there seemed to be some recursion caused by using cl-letf. Should be fixed by using cl-flet.

After invoking the debugger (i.e., M-x toggle-debug-on-entry RET beginning-of-buffer RET followed by M-x beginning-of-buffer RET), repeatedly stepping through (by pressing d) would recurse into our advice every time. I wasn't able to recreate the same recursion (with other test functions) outside of the debugger environment, so there is probably be another mysterious factor in the debugger that triggers the recursion.

@bmag bmag mentioned this pull request Apr 2, 2021
@wyuenho
Copy link
Collaborator

wyuenho commented Apr 3, 2021

This unfortunately doesn't work for my use case as we are back to pop-to-buffer not being able to pick a window wide enough again. Judging from the different heights of the windows in every invocation of debug, I don't think this advice has any effect whatsoever. With that said, I think it's best to revert #182 for now. I've since discovered that although it can hand control to purpose to pick a window, it doesn't pick a frame, so if toggle-debug-on-error is turned on and an error is thrown inside a small child frame like those made by company-box, the backtrace buffer is still popped up inside that tiny child frame. Picking a right window from the right frame needs a re-think.

@bmag
Copy link
Owner Author

bmag commented Apr 5, 2021

Ok, reverted (in 06f727e)

@bmag bmag closed this Apr 5, 2021
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

Successfully merging this pull request may close these issues.

2 participants