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

Complete Transient Stack Model #156

Closed
psionic-k opened this issue Sep 30, 2021 · 5 comments
Closed

Complete Transient Stack Model #156

psionic-k opened this issue Sep 30, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@psionic-k
Copy link
Contributor

Seems like there's some gaps in pre-commands or the stack model. There's two problems I can't quite get to work perfectly.

I've tried numerous combinations, transient--do-stay or invoking (transient-setup ...) to obtain a fake "return" behavior. I can't describe all of the edges that I observed while getting these examples this far along. Let's focus on two examples that are very close to a perfect "return" while still having access to parent arguments in the sub-prefix. Neither one fully succeeds.

In this first example, I want to be able to print the transient-toys-parent arguments from both the prefix and sub-prefix. To do this in the prefix, I need to use transient--do-call or else I get nil. If I set the transient-toys-msg-parent-args to :transient 'transient--do-call in the sub-prefix, the act of exporting the sub-prefix args makes the parent prefix args nil.

This reveals the frist edge case. I cannot access the parent arguments and suffix arguments at the same time. One or the other will be nil

     (transient-define-suffix transient-toys--wave ()
       "Wave at the user"
       (interactive)
       (message (propertize
                 (format "Waves at %s" (current-time-string))
                 'face 'success))
       (transient-setup 'transient-toys-wave-parent))

     (transient-define-suffix transient-toys--msg-parent-args (&optional args)
       "Show current infix args"
       :key "a"
       :description "show parent args"
       (interactive (list (transient-args 'transient-toys-wave-parent)))
       (message (concat (propertize "Current args: " 'face 'success)
                        (format "%s" args))))

     (transient-define-prefix transient-toys--wave-sub-prefix ()
       "Wave at the user"
       ["Sub Prefix"
        ["" "Options"
         ""
         ("-s" "switch" "--switch")]
        ["" "Commands"
         ("w" "wave" transient-toys--wave :transient transient--do-quit-one)
         (transient-toys--msg-parent-args :transient transient--do-stay)]]) ; change to transient--do-call to return `nil

     (transient-define-prefix transient-toys-wave-parent ()
       "Wave at the user"
       ["Parent prefix"
        ["" "Options"
         ""
         ("-s" "switch" "--switch")]
        ["" "Commands"
         ("w" "wave submenu" transient-toys--wave-sub-prefix :transient transient--do-call)
         (transient-toys--msg-parent-args :transient transient--do-call)]])  ; change to transient--do-stay to return `nil`
         
     (transient-toys-wave-parent)

In another example, we can construct a transient that returns to the parent prefix and keeps the same switch active, but it cannot message the parent arguments in both parent and sub-prefix.

     (transient-define-suffix transient-toys--wave ()
       "Wave at the user"
       (interactive)
       (message (propertize
                 (format "Waves at %s" (current-time-string))
                 'face 'success)))

     (transient-define-suffix transient-toys--msg-parent-args (&optional args)
       "Show current infix args"
       :key "a"
       :description "show parent args"
       (interactive (list (transient-args 'transient-toys-wave-parent)))
       (message (concat (propertize "Current args: " 'face 'success)
                        (format "%s" args))))

     (transient-define-prefix transient-toys--wave-sub-prefix ()
       "Wave at the user"
       ["Sub Prefix"
        ["" "Options"
         ""
         ("-s" "switch" "--switch")]
        ["" "Commands"
         ("w" "wave" transient-toys--wave :transient transient--do-quit-one)
         (transient-toys--msg-parent-args :transient transient--do-stay)]])

     (transient-define-prefix transient-toys-wave-parent ()
       "Wave at the user"
       ["Parent prefix"
        ["" "Options"
         ""
         ("-s" "switch" "--switch")]
        ["" "Commands"
         ("w" "wave submenu" transient-toys--wave-sub-prefix :transient transient--do-replace)
         (transient-toys--msg-parent-args :transient transient--do-call)]])

My expectation is that transient--do-replace would not result in a "return" from transient--do-quit-one. Also surprisingly, transient--do-replace leaves the parent arguments available while transient--do-call results in them being unavailable in the child and a failure of transient--do-quit-one to return.

Recommended Path to Fix

I believe access to arguments of multiple prefixes in one suffix or prefix body should be a thing. I'm not aware of any magit examples that do this. As far as I know, they only access their immediate parent's arguments. Exporting should not necessarily change the return value of other prefixes.

New pre-commands to clear things up:

  • transient--do-return should leave the arguments available and "return" to the most immediate predecessor that left itself available
  • transient--do-replace should not leave itself available for return
  • transient--do-call should leave itself available for return

transient--do-replace would become more like goto and could be used to implement recursive workflows with tail-call elimination transient--do-replace could also be used to enter a sequence of sub-prefixes that finally return to the main parent prefix
transient--do-call could be relied upon where a workflow was intended to be returned to.

Supporting calls creates a problem for accessing multiple prefixes. If a user uses transient--do-call on the same prefix several times, we would need a stack of states. While transient-args could just return the most recent, perhaps a new function is needed to get all available states from the stack.

I'll follow up later after I get some more time. It might be best to start with a total solution with less support for "goto" style navigation to avoid fragile flow control implementation. The concept in my head isn't obviously complete yet.

@psionic-k
Copy link
Contributor Author

The example case I'm working on is to use a sub-prefix like a builder or wizard. For example, I was going to build up a new suffix (a wiki example I was building). Instead of doing multiple readers, I thought using a prefix like a builder API might better handle the large amount of optional configuration.

From what I've read, sub-prefixes are mostly treated as if one-way, no-return. This means I would have to do my own plumbing both to get the data from the child and to reflect changes in the parent.

While it's natural that the user needs to go get arguments to interactive functions so that they can be used standalone, I don't think it's natural to have to resort to transient-values or shuttling data into globals. The current uses of scope are pretty simple and require a lot of manual work. I think we need a more uniform way for data to go from parent to child and return from child to parent.

A call stack and a data model for such a stack I think go together, and the outcome should be a little bit easier plumbing of data.

I'm going to open a separate issue for handling non-argument values, but it's related to growing beyond just wrapping CLI's.

@psionic-k psionic-k changed the title Persist arguments after export in nested transients Complete Transient Stack Model Oct 5, 2021
@psionic-k
Copy link
Contributor Author

Changed title. The very first job of this issue will be to complete the state machine implementation until:

  • Every state only expresses one idea
  • If a state has an enumeration of values, those values have no potential need to be simultaneously true
  • No implicit states or state inferences cause dilemmas when changing behavior on state changes

For example, transient--exitp looks like it's doing too much work. Comments such as these indicate that it in combination with implementation state is used to infer other states. https://github.com/magit/transient/blob/master/lisp/transient.el#L2030-L2034

it looks like an organically grown state machine that needs to be given a total model. Without fixing that up first, changes will be hard to make without introducing bugs or even to read in PR.

@psionic-k
Copy link
Contributor Author

https://github.com/magit/transient/wiki/Developer-Quick-Start-Guide#errata This example encountered another edge behavior when configuring a sub-prefix to "return". Not sure it's possible with current public or private function calls to set things up so that both paths in the child prefix body will "return" to the parent prefix.

@tarsius
Copy link
Member

tarsius commented Nov 27, 2021

Oh noos, you cancelled our talks? I was looking forward to that. ☹️

(And all the other canceled talks are those I was looking forward to the most; what's going on here?)

@tarsius tarsius added the enhancement New feature or request label Sep 14, 2022
@psionic-k
Copy link
Contributor Author

I have gotten a better understanding of both the stack mechanics and intent and think this issue has revealed itself to be poorly defined by me 🥉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants