Decompiler: Add code folding #8543
Conversation
Oops, missed a few...
There was a problem hiding this comment.
@LukeSerne It seems to be missing the keyboard method of activation. Is there a different solution for that?
This is more consistent with the UI used for the folding in the listing display.
This way, the same tokens will keep the same line number, even if earlier blocks have been folded.
After every syntax token that is an opening curly brace, an extra syntax token is created with the text "…". This token is collapsed by default, and uncollapsed when the other tokens in the block are collapsed and vice versa.
@Wall-AF Yeah, I noticed that too. It seems to not have been part of the PR I copied. I edited the opening post with a list of bugs / missing features of this PR and added the keyboard shortcuts to that as well. I saw that for the keyboard shortcuts in #4264, the solution the Ghidra team eventually ended up using was implemented in dd08e5c by creating separate |
Previously, if there were any off-screen collapsed blocks before the first visible line, the initial line number would be off, since it didn't take into account the previous folded blocks. This commit fixes that.
Now, DecompilerPanel#getLineNumber accounts for skipped lines due to collapsed blocks. As such, it will now return the line number shown in the margin at a given y-offset. If you want to get an index for the list returned by getLines(), just use `pixmap.getIndex(y).intValue()`.
This doesn't quite work 100% of the time, but it's close enough...
ryanmkurtz
added
Feature: Decompiler
Status: Triage
|
Hi, I partially reviewed the original PR in spbu-se. I wanted to point out that this implementation breaks the search functionality (and likely some other features) because the "text" of the folded part completely disappears. I have a private fix for this, but it's more of a hack since it's bad from both an architectural and performance perspective. |
|
Ah yes, you're right. Thanks for reporting it, I added it to the list. Most of this PR is not great from an architectural and performance perspective anyway, so feel free to publish your fix and I can add it to this PR. The implementation that keeps the cursor at the same token in the decompiler is also quite hacky and not too great from a performance point of view. It seems like the code folding in the listing that was recently merged instead uses an invisible font for the text that is folded away. Maybe this implementation should be more similar to that to break fewer implicit assumptions in the existing code that deals with the decompiler listing. Not sure how doable implementing that is though. |
Here you are I'll be glad if it will be useful. |
6 participants
Closes #1294
This PR copies over spbu-se#1 to the main repo. So thanks to @Gr-i-niy, @Wall-AF and @KirillSmirnov for writing the code and reviewing the original PR. I had no involvement in that PR - I just applied the PR's commits to the latest master, and made some small adjustments to fix merge conflicts.
This PR increases the left margin of the decompiler view and uses it to display "+" and "-" symbols at the start of code blocks. You can click those to fold/unfold code sections.
EDIT: After using this PR for a while, I noticed it still lacks a few features, which I'm planning to implement myself and adding to this PR. The current list:
Line numbers don't update when code is (un)foldedThe+and-symbols are inconsistent with the icons used for folding in the listing displayWhen a block is folded, the opening and closing brace are directly next to each other, without any indication of the collapsed tokensWhen (un)folding a block, the cursor remains at the same position, but the tokens have moved, so the selected token is now completely different.Before
After
After (folded)
