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

Expose asm.reloff.flags in Preferences #2244

Merged
merged 4 commits into from
Jun 16, 2020

Conversation

Surendrajat
Copy link
Member

Your checklist for this pull request

Detailed description
This PR adds asm.reloff.flags checkbox under Preferences -> Disassembly.

asm-reloff-flags

Test plan (required)
Go to Preferences -> Disassembly -> check Show offsets relative to a flag (asm.reloff.flags)

Closing issues

@karliss
Copy link
Member

karliss commented Jun 11, 2020

I am slightly worried about cluttering the first page with too many rarely used settings. @ITAYC0HEN what do you think?

@ITAYC0HEN
Copy link
Member

For this specific case it can use different ui components, for example

Show offset as relative to:    [x] Functions [ ] Flags
Show offset as relative to:    [ ] Functions [x] Flags
Show offset as relative to:    [x] Functions [x] Flags
Show offset as relative to:    [ ] Functions [ ] Flags

Or a combo box maybe (like I did with comments).

In general, I was thinking about it as well, and I see couple of ways we can go. Of course, some of them can be combined.

  1. First of all, the most complicated one, is refactoring of the whole thing into a modern setting widget (e.g vscode's). I think that Binja has something similar as well. This includes full descriptions, automatically creating the view etc. And of course with a quick filtering and a way to work together with a command pallette in the future.

  2. Turn the dialog into a dockable widget. This will allow more space and the items will look less crowded

  3. Use group boxes to gather related configurations together in a logical way. This will make it visually easier to navigate in the view.

  4. Use more tabs, and group configurations together in a logical way

  5. Use sub-items in the side bar to organize configurations in a logical way

  6. Use collapsible part at the bottom of the view to hide less-common config

@Surendrajat
Copy link
Member Author

Surendrajat commented Jun 12, 2020

@ITAYC0HEN For this change I like the first idea you suggested. It'll save up space and will be consistent with the current logic of graph.offset which won't really fit in the comboBox.
About the entire preference view change, yes, I like that idea too but I am not quite ready to pick it yet. If base is set, I can add things to it :)

@Surendrajat
Copy link
Member Author

This is how it looks like now:
update-asm-reloff-flags

@ITAYC0HEN
Copy link
Member

I think that "Show offsets relative to" can also be grayed out when asm.offset is disabled :)

@Surendrajat
Copy link
Member Author

@ITAYC0HEN Cool. It's done now. I've rebased and the build is broken but it should work.

Copy link
Member

@ITAYC0HEN ITAYC0HEN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now :)

@karliss karliss merged commit 7736088 into rizinorg:master Jun 16, 2020
@Surendrajat Surendrajat deleted the asm-reloff-flags branch June 16, 2020 11:00
fengjixuchui referenced this pull request in fengjixuchui/cutter Jun 16, 2020
Expose asm.reloff.flags in Preferences (radareorg#2244)
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.

None yet

3 participants