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(gui): Make code folding context menu actions appear #2234

Closed

Conversation

Mino260806
Copy link
Contributor

@Mino260806 Mino260806 commented Aug 1, 2024

The Issue

At first, I was going to mark this as a feature, only to find out this is already implemented. However, the current implementation is erroneous, and never adds the actions. Here's the problem

Code folding is disabled by default. Its value is set using AbstractCodeArea.setCodeFoldingEnabled().
However, AbstractCodeArea prebuilds the context menu in its constructor, fired in this line.

But superclass constructor is called before child constuctor, so AbstractCodeArea calls isCodeFoldingEnabled() before child gets to change it, so isCodeFoldingEnabled() below always returns its default value of false and code folding actions are never added.

@Override
protected void appendFoldingMenu(JPopupMenu popup) {
// append code folding popup menu entry only if enabled
if (isCodeFoldingEnabled()) {
super.appendFoldingMenu(popup);
}
}

My Solution

I thought the best way to add it is to add the folding menu actions once setCodeFoldingEnabled(true) is called. However RSyntaxTextArea.appendFoldingMenu(JPopupMenu) only adds the actions to the end, so I made a workaround that removes present actions after target index, adds folding menu, and then adds present actions back.

https://github.com/Mino260806/jadx/blob/9daac915ef08a048cb39581335839229ac9ecf93/jadx-gui/src/main/java/jadx/gui/ui/codearea/AbstractCodeArea.java#L142-L172

I'll open a PR in RSyntaxTextArea that creates RSyntaxTextArea.appendFoldingMenu(JPopupMenu, int), and we would fix this workaround once merged and released.

@skylot
Copy link
Owner

skylot commented Aug 1, 2024

Hm, I think we can run addWrapLineMenuAction inside createPopupMenu().
And all code for adding popup menu items should be done in createPopupMenu().
I will check this and prepare a fix.

@skylot
Copy link
Owner

skylot commented Aug 1, 2024

I commit a fix in fix-folding-menu branch, commit 8e00de3
It works fine for me. The only downside is that using this inheritance/override way it is harder to control order of items, but we still can insert something in the middle if needed. Anyway, current order looks fine to me.
@Mino260806 please check my commit, and you can use it or fix/adapt if you want 😄

@Mino260806
Copy link
Contributor Author

@skylot it's fine you can merge your branch, I was also hesitant about my approach

@Mino260806 Mino260806 closed this Aug 1, 2024
@Mino260806 Mino260806 deleted the code_folding_context_actions branch August 3, 2024 13:37
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