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

Add support for stacked list of modes #494

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

Conversation

leon-gh
Copy link
Contributor

@leon-gh leon-gh commented Jun 7, 2023

  • This is a hack around the v13.3 implementation of mode switching, which does not support stacked modes. It allows all sorts of neat tricks, such as multiple switch-to-previous mode calls, stacking of temporary modes and returning back, etc.

I am aware that the code quality might not be adequate - it's a hack. I wrote this to solve my issue and hopefully it gets enough attraction, so such feature can be implemented in r14 (if not already present). I'm also open to discussion how to implement it properly and can do it.

I tried to describe the benefits in doc/mode-stack/README.md

Cheers!

* This is a hack around the v13.3 implementation
  of mode switching, which does not support stacked modes.

  It allows all sorts of neat tricks, such as multiple
  switch-to-previous mode calls, stacking of temporary modes
  and returning back, etc.
@WhiteMagic
Copy link
Owner

That looks neat. A stack-based handling of mode switching makes sense and might work around some of the odd quirks with switching. I'm not too keen on the logic of handling the stack living inside the mode switch helper package itself as a bunch of free-standing functions with a global variable holding the state itself.

I can think of two directions that would work around this to make it better integrated.

  1. Unify the mode-switching actions, or at least some, and have them handle the logic
  2. Add a mode manager that holds the state
    The mode manager might actually be the one doing all the heavy lifting, and the actions themselves only call into it. Similar to what happens with the Macro action, which heavily relies on the MacroManager to execute the macros themselves.

As with other pull requests, I'm not likely to merge it into the code base as anything R13 I no longer touch due to the very limited amount of time I have to work on Gremlin at all. Though the idea is nice and something that would be a good addition to R14.

* Proper removal of non-temporary mode from mode stack in scenarios when
   temporary mode switch is needed to access the switch to previous mode
   binding.
@leon-gh
Copy link
Contributor Author

leon-gh commented Nov 6, 2023

I can think of two directions that would work around this to make it better integrated.

1. Unify the mode-switching actions, or at least some, and have them handle the logic

2. Add a mode manager that holds the state
   The mode manager might actually be the one doing all the heavy lifting, and the actions themselves only call into it. Similar to what happens with the Macro action, which heavily relies on the MacroManager to execute the macros themselves.

It feels that ModeManager approach would be the the cleaner and then actions just calling it.

As with other pull requests, I'm not likely to merge it into the code base as anything R13 I no longer touch due to the very limited amount of time I have to work on Gremlin at all. Though the idea is nice and something that would be a good addition to R14.

I've tried to get R14 to work, but was unable - UI was very strange and it felt like work-in-progress. If you don't have time for R13 and I do the ModeManager implementation, could you just pull it in then so it does not have to be forked release?

@WhiteMagic
Copy link
Owner

WhiteMagic commented Nov 20, 2023

I agree I personally would likely do a manager for modes as that would allow abstracting away the logic into an internal component that has an API used by the actions, allowing for internal changes without breaking things.

R14's status is very fluctuating depending on what aspect I worked on last. At this stage it's not a good idea yet to try to code anything against it as core things might still change. I think that most of the profile related aspects are now settled which I should know once I get around to implementing axis splitting and merging actions. Currently there is also no notion of mode handling in there yet.

As I mentioned before I'm not likely going to merge anything into the R13 branch, mainly because if I do one thing then there are 20 other things that are also things I should do. There are realistically quite a few tiny bug fixes that would fix minor but annoying bugs which should be merged before a new feature like a mode manager. Though I've not done that either because I would not merge and release a new build without properly checking everything works. Which with the limited time I get to spend on Gremlin would effectively stall progress on R14 even more than it already is.

Though if you're interested in giving implementing a mode manager a shot R13 should be ok enough as a playground as I don't see modes themselves changing much in R14. There still will be single inheritance only with the usual switching operations. As the mode manager would be disconnected from the actions themselves that aspect would move across to R14 without too many issues I suspect, as opposed to actions which changed quite substantially.

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.

2 participants