-
Notifications
You must be signed in to change notification settings - Fork 47
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
base: r13
Are you sure you want to change the base?
Conversation
* 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.
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.
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.
It feels that ModeManager approach would be the the cleaner and then actions just calling it.
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? |
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. |
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!