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

Enhanced Decompiler Widget with new Dedicated Context Menu and Actions #2391

Merged
merged 10 commits into from Aug 18, 2020

Conversation

NirmalManoj
Copy link
Member

@NirmalManoj NirmalManoj commented Aug 14, 2020

Your checklist for this pull request

Detailed description
As part of the Improving Decompiler Widget project, we have made many improvements to the decompiler widget and have made a new dedicated context menu for the decompiler in the past couple of months. Most of these changes were committed to the decompiler-refactoring branch. This PR is to merge all of the new features to master.

The important changes and the corresponding PRs are listed below.

  1. Dedicated decompiler context menu (skeleton) with an action to copy selected code #2256 - Dedicated decompiler context menu (skeleton) with an action to copy selected code
  2. Breakpoint Menu and Debug Menu in the Decompiler Context Menu #2260 - Breakpoint Menu and Debug Menu in the Decompiler Context Menu
  3. Comment Menu for the Decompiler Context Menu #2265 - Comment Menu for the Decompiler Context Menu
  4. Right-click updates cursor and opens context menu #2281 - Intercept right-click event in the decompiler widget
  5. Action to rename functions in the decompiler context menu #2286 - Action to rename functions in the decompiler context menu
  6. Show-in menu for references and add/rename/delete name for global variables #2295 - Some global variable actions and Show in action
    * Show in action for global variables and functions
    * Copy address of global variable or function referenced by the cursor selection
    * Rename global variable
  7. Display jumping fix useless decompilation #2351 - Fix display jumping in the widget for every decompilation. Reduce unnecessary decompilations.
  8. X-Refs for references #2352 - Xrefs action for references
  9. Edit/Rename Variables Actions for function variables #2357 - Edit/Rename Variables Actions for function variables
  10. Documentation + Clean up #2374 - Documenting code and final cleanup

Test plan (required)

  1. Fetch the latest master of r2ghidra-dec and the current commit in the latest radare2 submodule.
  2. Go through all the linked PRs and test all the features implemented by each PR. Just see if all of them are working as expected.
  3. Try to check if anything else is broken by these changes using Cutter for something you normally do.

Closing issues
closes #1764

src/widgets/HexdumpWidget.cpp Outdated Show resolved Hide resolved
@NirmalManoj
Copy link
Member Author

NirmalManoj commented Aug 15, 2020

Please have a look at the commit descriptions also and think if I should improve it by adding or removing anything.

@NirmalManoj NirmalManoj requested a review from karliss Aug 15, 2020
NirmalManoj added 8 commits Aug 15, 2020
Update cursor before opening context menu for right-click
* Show in action for global variables and functions
* Copy address of global variable or function referenced by the cursor selection
* Rename global variable
* save scroll position and reset to that if the newly decompiled function is the same as the previous one

* instructionChanged signal replaced completely by breakpointsChanged in toggle/add breakpoint functions.

* removed addbreakpoint(QString) and toggleBreakpoint(QString)
* X-Refs for references (functions, global variables, constant variables with addresses) in the decompiler.
@NirmalManoj NirmalManoj force-pushed the decompiler-refactoring branch 2 times, most recently from 8f711da to 5a6baa3 Compare Aug 15, 2020
src/core/Cutter.cpp Outdated Show resolved Hide resolved
NirmalManoj added 2 commits Aug 17, 2020
* Edit Function Variables Action

* Rename Function Variables Action

* CutterCore::renameFunctionVariable
Copy link
Member

@karliss karliss left a comment

I would say changes are good enough for merging. The old context menu might have had a few more options available but I think the more precise context sensitivity outweighs it. Also just because there were more actions doesn't mean they were working reliably or acted on on the item user would expect.

There is always room for further improvements, but I didn't observe anything that would be a blocker for merging this.

@NirmalManoj
Copy link
Member Author

NirmalManoj commented Aug 18, 2020

Feels fantastic to have done so much impactful work! Thanks a lot for your support and guidance @karliss @ITAYC0HEN @thestr4ng3r

@NirmalManoj NirmalManoj merged commit b7d1059 into master Aug 18, 2020
9 checks passed
@NirmalManoj NirmalManoj moved this from In progress to Done in Improving Decompiler Widget (GSoC) Aug 18, 2020
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.

Use custom context menu in Decompiler instead of DisassemblyContextMenu
3 participants