Skip to content

Conversation

@NicoWeio
Copy link

@NicoWeio NicoWeio commented Nov 17, 2022

My suggested behavior is setting the seed to the just-undone number or (just-redone + 1), respectively.

I'm not familiar with this codebase at all, so my approach might be quite ugly – but it works and currently, no better solution (as in “a PR”) seems to exist.

Fixes #308

@DamirPorobic
Copy link
Member

What is the motivation for having this behavior? Do you have scenario where such a behavior is required?

@NicoWeio
Copy link
Author

I think #308 describes it pretty well.
The suggested behavior is not strictly required, but more efficient and IMO more intuitive.
If I press Ctrl+Z, I expect all state to be “reset”.
However, currently, adding a number annotation, undoing, then adding one again, would result in a larger annotation number each time you do it.

@DamirPorobic
Copy link
Member

That makes sense. Still I don't like the if there, would love to see some inheritance helping us out. Could you add some simple factory that returns one of two types of AddCommands, for NumberTools a AddAnnotationNumberCommand and for all other the usual AddCommand. The AddAnnotationNumberCommand inherits from AddCommand and does all the same (by calling the base) + resets the seed. I know it's a bit more code but I think it's the right way to do it.

@NicoWeio
Copy link
Author

I absolutely agree. As I said, this was only a quick draft.

Unfortunately, I'm not that accustomed to C++ and am currently struggling with referencing AddCommand from AddAnnotationNumberCommand. For some reason, all I get is expected class-name before »{« token.

It might take me some weeks or months until I can allocate more time to this, so if you or somebody else would like to, you are more than welcome to take over.

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.

Undoing a Number annotation does not decrease Number Seed

2 participants