wryun / es-shell

es: a shell with higher-order functions
http://wryun.github.io/es-shell/
Other
313 stars 26 forks source link

Make `$status` available in the interactive loop #66

Closed jpco closed 1 week ago

jpco commented 11 months ago

The creatures feep, but I think this creature is well-justified and expected by the "average" user.

wryun commented 11 months ago

Given that it's only in the interactive loop, seems mostly harmless, but... let me sit on this for a bit. Given $status is also referenced in the exception handling, there's probably some edge case where the behaviour is different (e.g if you redefine status in fn-dispatch or fn-prompt and then somehow eof), though I don't think it would cause actual harm. I guess a safe option would be to keep the existing $result variable for the exception handling and never use $status.

My preference as a user for this sort of thing is to surface it in the prompt, which I guess this also enables.

hyphenrf commented 11 months ago

for prior attempts context #33

I agree that the only real value of this is in interactive use, in shell and in the prompt, as there's no "checking it after the fact" in scripts -- can just capture the evaluation result as one normally does. As a side-effect of the way it was done in that issue linked above, variable bindings were also reflected in $status, meaning

; x = 1
; echo $status
1

I'm not commenting from my personal machine, so I can't test whether it's the case here.

Here's a use-case of $status from my .esrc

# override prompt to take a different color on nonzero
# TODO: x = 1 returns 1; and is thus a "nonzero status"
#
let (ogprom = $prompt)
fn %prompt {
   if { nonzero $status } {
      prompt = <={ rgbcol $ogprom(1) 255 55 55 } $ogprom(2 ...)
   }{
      prompt = $ogprom
   }
}
jpco commented 11 months ago

hyphenrf: I didn't see there was previous context here -- thanks for posting that. I should take a closer look at prior art before throwing ideas around. Nice that we came up with essentially exactly the same change. And yes, you're right about assignments setting $status.

wryun: I think I see what you're talking about? Something like this?

let (pr = $fn-%prompt; fuse = three two one)
fn %prompt {
    if {~ $fuse ()} {
        status = 'Boom!'
        throw eof
    } {
        fuse = $fuse(2 ...)
        $pr $*
    }
}

I guess that's bad, but IMO the main problem there is %prompt having no dedicated exception catcher protecting the interactive loop. For example, I think this is much more unpleasant, and works at HEAD:

let (pr = $fn-%prompt; fuse = three two one)
fn %prompt {
    if {~ $fuse ()} {
        throw error %prompt 'Boom!'
    } {
        fuse = $fuse(2 ...)
        $pr $*
    }
}
wryun commented 2 months ago

Looking at this again, the other thing that makes me a bit wary is that it might be confusing to users to have status only in the interactive loop and bqstatus everywhere.

This made me look at $status in rc. Given that es seems to have inherited bqstatus from rc, if we're going to have a status maybe it would be nice to have an rc style status?

e.g.

; false | true | true | false
; echo $status
1 0 0 1

But in case it wasn't already clear, as far as I'm concerned if you're keen enough about something feel free to merge it; active maintainer wins IMO!

jpco commented 1 month ago

I think you have a good point about the asymmetry of it -- that seems like a bummer for people who didn't opt into that asymmetry on purpose. And I'm definitely not interested in adding $status to scripts. I prefer the explicit control flow and, from reading the rc list, it seems like "magic variables" are generally a rich source of confusion.

One thing about es that I'm a little dissatisfied with is the "big lift" of redefining %interactive-loop for little changes. In this case you could probably do a %dispatch hack, but that has its own trickiness to work around es' own resetting of %dispatch.

I think I might close this PR and instead propose documenting this as a caveat in the man page.

wryun commented 1 month ago

Hmm, could add a hook function to interactive-loop or dispatch? e.g. if a function %dispatch-result is defined, call it with the result as the argument?

tbh, it still has those problems you identify with explicit control flow and still pollutes the global namespace, but it somehow seems cleaner and more functional to me.

jpco commented 1 month ago

Maybe. Thinking about it, I do consider addressing the flexibility of the REPL to be better than what I've proposed in this PR (generality and extensibility is the es way!) But it's also a much bigger design task.

Personally, the most common things I think about changing in %interactive-loop are:

Trying it out -- with a hacked es that does the above, putting the following by itself in .esrc actually does work (and es -x and friends still do too):

let (d = $fn-%dispatch)
fn %dispatch {status = <={$d $*}}

This simple version isn't quite as sophisticated as would be preferred -- no dynamic scoping for status, for example, and doesn't account for %interactive-loop vs. %batch-loop -- but simple hacks have simple behavior, and it's reasonably easy to imagine how to make this more sophisticated. Maybe it would be worth a bit of thought on how to protect %batch-loop by default. Maybe %batch-loop just calls %es-dispatch directly -- that way it's still hackable, but it's harder to do so, and that seems appropriate for %batch-loop, which usually shouldn't be messed with nearly as much as %interactive-loop

This is, to me, the largest open question. These are the least extensible part of %interactive-loop right now -- nestled inside nontrivial, load-bearing control flow, with no hook functions available to help -- and they seem to me to be entirely reasonable to want to extend (for example: on my own machine I have installed a sigwinch handler that maintains the LINES and COLUMNS variables in the environment).

I personally have my %interactive-loop set up to use rc-like fn-sigwinch-style handlers, and they're pretty nice. But for the distributed es that feels like it's imposing too much of a specific pattern. On the other hand, I don't think I'd want to simply change the entire %interactive-loop exception handler to be a big hook function -- that seems like it would make it too easy to break eof while trying to install some at-exit handler.

Thoughts on any of this?


As an aside, here's my favorite way to edit %dispatch to set status in es today. It's certainly tricky (really depends on all those distinctions between local and let!) but I think it's correct, and it's fairly concise at least.

let (il = $fn-%interactive-loop)
fn %interactive-loop {
    let (d = $fn-%dispatch)
    local (
        noexport = $noexport status
        status = <=true
        fn %dispatch {status = <={$d $*}}
    ) $il $*
}
jpco commented 1 week ago

With canonical extensions (#131), this can be closed.