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 layout save and restore feature (#1515) #1538

Merged
merged 8 commits into from
Jun 18, 2019
Merged

Fix layout save and restore feature (#1515) #1538

merged 8 commits into from
Jun 18, 2019

Conversation

optizone
Copy link
Contributor

Detailed description

Test plan (required)

Closing issues

src/core/MainWindow.h Outdated Show resolved Hide resolved
src/core/MainWindow.cpp Outdated Show resolved Hide resolved
src/widgets/DisassemblerGraphView.cpp Outdated Show resolved Hide resolved
src/widgets/DisassemblyWidget.cpp Outdated Show resolved Hide resolved
src/widgets/GraphWidget.cpp Outdated Show resolved Hide resolved
src/widgets/HexdumpWidget.cpp Outdated Show resolved Hide resolved
src/widgets/MemoryDockWidget.h Outdated Show resolved Hide resolved
src/core/MainWindow.cpp Outdated Show resolved Hide resolved
src/common/CutterSeekable.cpp Outdated Show resolved Hide resolved
src/core/MainWindow.cpp Show resolved Hide resolved
Copy link
Member

@thestr4ng3r thestr4ng3r left a comment

Choose a reason for hiding this comment

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

I'm not yet convinced of using class names for the object names. I would rather add a static method to DisassemblyWidget, GraphWidget and HexdumpWidget that looks something like this:

static QString getWidgetTypeId() { return "Disassembly"; }

Then you can use DisassemblyWidget::getWidgetTypeId() in all the places where the class name is used currently. This way we can also keep the previous states and no migration should be necessary.

src/core/MainWindow.cpp Outdated Show resolved Hide resolved
src/core/MainWindow.cpp Outdated Show resolved Hide resolved
src/widgets/MemoryDockWidget.cpp Outdated Show resolved Hide resolved
src/Main.cpp Outdated Show resolved Hide resolved
src/core/MainWindow.cpp Outdated Show resolved Hide resolved
@optizone
Copy link
Contributor Author

optizone commented Jun 15, 2019

Briefly. Qt uses object names to save state of mainwindow. Now we use names like "Disassembly", "Graph" etc. If there are few objects with such names, Qt can not distinguish ones. So some sort of ID is obviously needed. I chose to use "ClassName ID" pattern because of more convenient restoring (simply use first word from obj name to find constructor).

src/core/MainWindow.cpp Outdated Show resolved Hide resolved
src/Main.cpp Outdated Show resolved Hide resolved
src/core/MainWindow.cpp Outdated Show resolved Hide resolved
src/widgets/DisassemblyWidget.cpp Outdated Show resolved Hide resolved
src/widgets/HexdumpWidget.cpp Outdated Show resolved Hide resolved
src/widgets/GraphWidget.cpp Outdated Show resolved Hide resolved
Copy link
Member

@thestr4ng3r thestr4ng3r left a comment

Choose a reason for hiding this comment

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

Apart from these little things, lgtm! I did some tests with old configs, clean config, etc. and everything worked fine.

src/widgets/DisassemblyWidget.cpp Outdated Show resolved Hide resolved
src/widgets/GraphWidget.cpp Outdated Show resolved Hide resolved
src/widgets/HexdumpWidget.cpp Outdated Show resolved Hide resolved
@thestr4ng3r
Copy link
Member

Very nice, now it really enhances the whole architecture. Great job and thanks for keeping at it for so long!

@thestr4ng3r thestr4ng3r merged commit 06aceaf into master Jun 18, 2019
@thestr4ng3r thestr4ng3r deleted the layout branch June 18, 2019 13:02
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

2 participants