Skip to content

Undo project - #3995

Open
rozyczko wants to merge 462 commits into
mainfrom
undo_project
Open

Undo project#3995
rozyczko wants to merge 462 commits into
mainfrom
undo_project

Conversation

@rozyczko

@rozyczko rozyczko commented Jun 5, 2026

Copy link
Copy Markdown
Member

Description

Added Undo/Redo functionality, tied to the GUI taskbar components.
This works in all perspectives and reverses any user input in any textfield.

This PR is against main, since I think the feature is important to be added to the next release.

The PR has a bunch of unrelated changes coming from rebasing against the recent main.

How Has This Been Tested?

Tested locally, added a number of unit tests.

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • There is a chance this will affect the installers, if so
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)
    • Wheels installer (GH artifact) has been tested (installed and worked)

Licensing (untick if necessary)

  • The introduced changes comply with SasView license (BSD 3-Clause)

Thought this was already here but apparently not.
Old way was throwing errors because symbol lookup doesn't have any of
the inverse units (or indeed units of any other dimension).
accepted is a slot not a value.
Was previously always returning an empty set.
@rozyczko rozyczko added Enhancement Feature requests and/or general improvements Being Tested When an issue is done but not ready to be closed. labels Jun 5, 2026
@rozyczko
rozyczko marked this pull request as ready for review June 8, 2026 08:56
@rozyczko rozyczko removed the Being Tested When an issue is done but not ready to be closed. label Jun 8, 2026
@rozyczko
rozyczko requested a review from krzywon June 8, 2026 12:40

@krzywon krzywon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall, this is looking very promising, but I'm seeing some undo/redo that aren't repeatable. I'll take a deeper dive into the code soon, but I did make a couple of suggestions.

After changing the scale value to 0.5, then clicking compute:
Image

Clicking undo shows no plot changes, but the scale value changes:
Image

Undo again and the plot changes:
Image

Redo twice to get back to the head of the stack and the scale is back to 0.5, but the plots aren't updated:
Image

Comment thread src/sas/qtgui/Utilities/Preferences/GeneralPreferencesWidget.py Outdated
Comment thread src/sas/qtgui/Perspectives/UndoRedo.py Outdated
Comment thread pyproject.toml
@rozyczko
rozyczko requested a review from krzywon June 19, 2026 06:42

@krzywon krzywon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The issues I was seeing previously are now resolved. This seems to work pretty well and will be a welcome feature!

Two things that need resolving before merging:

  1. Is this meant to point toward main or rafactor_24? This is pulling in the ASCII dialog and MetaData GUI is why I ask.
  2. There is a merge conflict in config.py.

@rozyczko

Copy link
Copy Markdown
Member Author

Thanks, @krzywon !

  1. Is this meant to point toward main or rafactor_24? This is pulling in the ASCII dialog and MetaData GUI is why I ask.

It' supposed to be merged with main since the functionality isn't really a refactor but a new feature.

  1. There is a merge conflict in config.py.
    Fixed!

@jamescrake-merani

Copy link
Copy Markdown
Contributor

I think this branch is based on the refactor branch even though this PR is pointing to main which is why we're seeing a lot of lines in the diff that are not related to this change.

@rozyczko would you be able to rebase this branch onto main? It'll make it easier to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Feature requests and/or general improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants