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

Retain file information when closing and reopening #176

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

camila314
Copy link
Contributor

Previously, closing and reopening the program meant having to re-save the opened file and overwrite the file. This retains the opened file so you no longer have to go through that process.

Also changed "Clear the watch list" to "Reset watch list" and made it discard the saved file information.

@dreamsyntax
Copy link
Collaborator

dreamsyntax commented Aug 25, 2024

I like the intent behind this, but I still would prefer something more like this: #121
Your change is basically what I want, but I also wanted a way to toggle the modes, incase working in a scratch space.

The clear/reset change sounds good.

As for the commits, if you rebase this branch on current main/master it should resolve the extra commits.

@camila314
Copy link
Contributor Author

the best system would be to make it document-based (like Bit Slicer for macOS) but i fear i don't know enough about qt to make that change, maybe some day though

@camila314 camila314 reopened this Aug 26, 2024
@dreamsyntax
Copy link
Collaborator

Just to be clear, so we don't break the flow of existing users I would want
#121 (comment)

File > [x] Autoload Last File
     > [x] Autosave Last File

Something like that in the PR that enables this behavior.
It could just be one toggle.
File -> [x] Auto Load/Save File

@dreamsyntax
Copy link
Collaborator

Bumping.

@cristian64 thoughts on just merging as is? I like the general idea even if I prefer optionally maintaining the old way.

@@ -267,7 +267,7 @@ void MemWatchWidget::onMemWatchContextMenuRequested(const QPoint& pos)
contextMenu->addSeparator();
}

if (!node || node->isGroup())
if (node == m_watchModel->getRootNode() || node->isGroup())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally, this change (which relates to #175) should land in its own PR.

@@ -853,6 +853,7 @@ void MemWatchWidget::clearWatchList()
return;

m_watchModel->clearRoot();
m_watchListFile = "";
Copy link
Collaborator

Choose a reason for hiding this comment

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

To avoid constructing another object for the sake of clearing the member:

- m_watchListFile = "";
+ m_watchListFile.clear();

.DS_Store
.cache
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, what is .cache?

And, as always, these unrelated changes should at the very least have their own commit and description. Perhaps it can even have a dedicated PR to avoid getting the change blocked by other potentially controversial changes.

@cristian64
Copy link
Collaborator

Bumping.

@cristian64 thoughts on just merging as is? I like the general idea even if I prefer optionally maintaining the old way.

I may be a bit out of the loop, but I'm not entirely sure what these are trying to accomplish. By looking at the diff, I can't see how the watch list file path is consumed on startup.

@dreamsyntax
Copy link
Collaborator

@camila314 following up if you plan to make changes, I'm going to release another patch version soon

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.

3 participants