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

QInstruction Click Handler Bugs #124

Closed
sraboy opened this issue Dec 2, 2019 · 3 comments · Fixed by #125
Closed

QInstruction Click Handler Bugs #124

sraboy opened this issue Dec 2, 2019 · 3 comments · Fixed by #125

Comments

@sraboy
Copy link
Contributor

sraboy commented Dec 2, 2019

  1. Typo: handle_insn_click vs PluginManager.handle_click_insn
  2. Also, the conditionals here consume the click event if it's just a normal left-click, so the plugins are never called. I use every selection of an address to track user activity, telemetry-style, so I can't rely on InfoDock.selected_insns (which doesn't tell me which was just clicked, just all of which are currently selected; it also doesn't give me right clicks).

elif self.workspace.plugins.handle_insn_click(self, event):

@rhelmot
Copy link
Member

rhelmot commented Dec 3, 2019

I saw in your fork a commit that added an attribute to the am_event for updating selected_insns to say which instruction in particular was clicked. That would be okay to add.

idk how I feel about giving plugins the ability to override the default behavior for normal left and right clicks

rhelmot added a commit that referenced this issue Dec 3, 2019
@rhelmot
Copy link
Member

rhelmot commented Dec 3, 2019

generally I support adding whatever metadata you like to am_events to support whatever behavior you want or could possibly foresee. all the callbacks are supposed to take kwargs for a reason!

@sraboy
Copy link
Contributor Author

sraboy commented Dec 3, 2019

Totally forgot I did that. PR inbound!

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 a pull request may close this issue.

2 participants