Very simple popen() code request, ground-shaking functionality openned by it

Very simple popen() code request, ground-shaking functionality openned by it

[Sebastian Gniazdowski]

Hello!

Git default progress indicator for clone is very unattractive, IMO. It does its job in providing all the operation details very well, but I bet most of users strongly dream about a gauge box!

Have a look at my gauge box constructed as git-stderr pipe script: https://asciinema.org/a/202401

The main point of my feature request is: git can add core.progress_pipe option, where e.g. `/usr/local/bin/mygauge' will be set (a script like the one in the asciinema), and then simply do the ground-school-known `popen("/usr/local/bin/mygauge","r+")', and write **unchanged current-progress data** to the pipe, then read from the pipe and forward to `stderr', where the progress normally lands in.

This will allow users to free their creativity and provide probably dozens of custom Git progress bars.

--

Best regards,
Sebastian Gniazdowski
Blog: http://zdharma.org

Re: Very simple popen() code request, ground-shaking functionality openned by it

[Jeff King]

On Fri, Sep 21, 2018 at 09:34:14AM -0400, Sebastian Gniazdowski wrote:

> Git default progress indicator for clone is very unattractive, IMO. It
> does its job in providing all the operation details very well, but I
> bet most of users strongly dream about a gauge box!
> 
> Have a look at my gauge box constructed as git-stderr pipe script:
> https://asciinema.org/a/202401
> 
> The main point of my feature request is: git can add
> core.progress_pipe option, where e.g. `/usr/local/bin/mygauge' will be
> set (a script like the one in the asciinema), and then simply do the
> ground-school-known `popen("/usr/local/bin/mygauge","r+")', and write
> **unchanged current-progress data** to the pipe, then read from the
> pipe and forward to `stderr', where the progress normally lands in.
> 
> This will allow users to free their creativity and provide probably
> dozens of custom Git progress bars.

I don't personally feel that the existing progress bar is that bad, but
if anybody wants to pursue this, I think the most sensible path is:

  1. Add a trace_key for sending machine-readable progress output to a
     descriptor or file. E.g., via setting GIT_TRACE_PROGRESS=2 in the
     environment.

  2. Teach the trace code to open a command for piping, so that you
     could do something like GIT_TRACE_PROGRESS='|mygauge'.

That would make your use case work, and I think many other use cases
would benefit from both of those features independently.

-Peff

Re: Very simple popen() code request, ground-shaking functionality openned by it

[Ævar Arnfjörð Bjarmason]

On Fri, Sep 21 2018, Jeff King wrote:

> On Fri, Sep 21, 2018 at 09:34:14AM -0400, Sebastian Gniazdowski wrote:
>
>> Git default progress indicator for clone is very unattractive, IMO. It
>> does its job in providing all the operation details very well, but I
>> bet most of users strongly dream about a gauge box!
>>
>> Have a look at my gauge box constructed as git-stderr pipe script:
>> https://asciinema.org/a/202401
>>
>> The main point of my feature request is: git can add
>> core.progress_pipe option, where e.g. `/usr/local/bin/mygauge' will be
>> set (a script like the one in the asciinema), and then simply do the
>> ground-school-known `popen("/usr/local/bin/mygauge","r+")', and write
>> **unchanged current-progress data** to the pipe, then read from the
>> pipe and forward to `stderr', where the progress normally lands in.
>>
>> This will allow users to free their creativity and provide probably
>> dozens of custom Git progress bars.
>
> I don't personally feel that the existing progress bar is that bad, but
> if anybody wants to pursue this, I think the most sensible path is:

I don't think it's bad either, but one thing that's really neat about
Sebastian's suggestion is that it's using some UTF-8 terminal ASCII art
to render an actual progress bar.

It would be very cool if we had some "we can use non-ASCII for such art"
core.* option, similar to how we pushed the boundaries with what was
acceptable with pagers and having colors on by default (and perhaps we
could even turn such a non-ASCII mode on by default eventually...).

Duy's
https://public-inbox.org/git/20180920161928.GA13379@duynguyen.home/ is
another recent thing that reminded me of this, i.e. that suggested
"\\|/-" spinner could be made much neater with non-ASCII.

>   1. Add a trace_key for sending machine-readable progress output to a
>      descriptor or file. E.g., via setting GIT_TRACE_PROGRESS=2 in the
>      environment.
>
>   2. Teach the trace code to open a command for piping, so that you
>      could do something like GIT_TRACE_PROGRESS='|mygauge'.
>
> That would make your use case work, and I think many other use cases
> would benefit from both of those features independently.

Yup, that's all sensible, and would be great both for this and other
stuff if we wanted true extensibility for this sort of thing.

I'll just add that a 3rd thing that would also make sense would be to
add a feature to configure the value of these GIT_TRACE_*=* variables
via the .gitconfig, that's been suggested before (too lazy to dig up a
ML archive reference), and would make this as easy to configure as
Sebastian's suggestion.

Re: Very simple popen() code request, ground-shaking functionality openned by it

[Jeff King]

On Sat, Sep 22, 2018 at 01:30:36AM +0200, Ævar Arnfjörð Bjarmason wrote:

> >> This will allow users to free their creativity and provide probably
> >> dozens of custom Git progress bars.
> >
> > I don't personally feel that the existing progress bar is that bad, but
> > if anybody wants to pursue this, I think the most sensible path is:
> 
> I don't think it's bad either, but one thing that's really neat about
> Sebastian's suggestion is that it's using some UTF-8 terminal ASCII art
> to render an actual progress bar.

Yeah. I don't care myself, but I'm not opposed to somebody trying to
spruce up the in-code bar, as long we can still handle the lowest common
denominator cleanly (and remember that includes passing progress bars
back over the remote sideband).

> >   1. Add a trace_key for sending machine-readable progress output to a
> >      descriptor or file. E.g., via setting GIT_TRACE_PROGRESS=2 in the
> >      environment.
> >
> >   2. Teach the trace code to open a command for piping, so that you
> >      could do something like GIT_TRACE_PROGRESS='|mygauge'.
> >
> > That would make your use case work, and I think many other use cases
> > would benefit from both of those features independently.
> 
> Yup, that's all sensible, and would be great both for this and other
> stuff if we wanted true extensibility for this sort of thing.
> 
> I'll just add that a 3rd thing that would also make sense would be to
> add a feature to configure the value of these GIT_TRACE_*=* variables
> via the .gitconfig, that's been suggested before (too lazy to dig up a
> ML archive reference), and would make this as easy to configure as
> Sebastian's suggestion.

Heh, I almost included that in my original mail, but didn't want to get
into the tangle of secondary concerns. But since you mention it... :)

One thing to watch out is that (2) and (3) combined may mean executing
arbitrary code specified in the .git/config of an untrusted repository.
This is already the case for many commands, but we've specifically tried
to avoid it with git-upload-pack, making it safe to "git fetch" out of
an untrusted repository. It's almost certain that you'd be able to
trigger trace code from it.

There are a number of solutions floating around. We already have some
upload-pack config which is smart enough to realize when its source is
in-repo and handle it appropriately, and we've talked on the list about
having some "I don't trust this repo" environment variable that would
make Git operate in a more restricted way. I don't think we need to hash
out the solution here, but I just want to mention that it's a thing that
would have to dealt with before adding those two features.

(Actually, I guess you could argue that even reading existing trace
specs out of config is potentially dangerous, since you can append to
arbitrary files).

-Peff

Re: Very simple popen() code request, ground-shaking functionality openned by it

[Sebastian Gniazdowski]

On 22 września 2018 at 01:30:36, Ævar Arnfjörð Bjarmason (avarab@gmail.com) wrote: 
> Duy's  
> https://public-inbox.org/git/20180920161928.GA13379@duynguyen.home/ is  
> another recent thing that reminded me of this, i.e. that suggested  
> "\\|/-" spinner could be made much neater with non-ASCII.  

Here is a IMO very large collection of spinner-like unicode animations: 

https://asciinema.org/a/ex8z3z6d5m7uv4buww0o2qeq2 

This comes from Zsh world, it's a plugin with spinners to use in Zsh scripts. I've never managed to see even 1/3 of them. 

> I'll just add that a 3rd thing that would also make sense would be to  
> add a feature to configure the value of these GIT_TRACE_*=* variables  
> via the .gitconfig, that's been suggested before (too lazy to dig up a  
> ML archive reference), and would make this as easy to configure as  
> Sebastian's suggestion.  

Yes git config setting of this is most convenient IMO, most expected to occur in ~/.gitconfig, in which it would be set once to a favourite gauge-box script, and rather long long before looking at this part of config again. Or maybe in the beginning, dawn of such gauge-scripts (if there actually would be any new group of such scripts; but as it's a quite broad problem (see last `PS.' paragraph), then who knows), when some unstable gauge-box would be breaking login/passwords prompt (but that's stdout not stderr, shouldn't go through gauge-box-script) or "fatal: ..." messages, etc., user might be disabling it temporarily or choosing an alternate gauge-box solution, editing the config option ;) So not rarely edited in the beginning. (I don't know how much important would a fancy gauge box be for a regular user; I can tell it would be quite important to me). I think this is more convenient and clean than `export GIT_*' in .bashrc/.zshrc, rarely edited, just sitting there. 

Guys you seem to like the idea, I hope someone will approach the coding! 

PS. There's much room for improvement in the git-process-output.zsh in the Asciinema video, gauge scripts won't be simple. In general, the number of 0..100% sequences (like: compressing, resolving, etc. – they all go 0 to 100% on their own) should be somehow predicted (does Git know this in advance?) and the gauge should be divided into that many segments, each filling up per one corresponding 0..100% sequence, together forming single global 0..100% gauge.

--  
Sebastian Gniazdowski 
News: https://twitter.com/ZdharmaI 
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin 
Blog: http://zdharma.org

Re: Very simple popen() code request, ground-shaking functionality openned by it

[Duy Nguyen]

On Sat, Sep 22, 2018 at 1:30 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Duy's
> https://public-inbox.org/git/20180920161928.GA13379@duynguyen.home/ is
> another recent thing that reminded me of this, i.e. that suggested
> "\\|/-" spinner could be made much neater with non-ASCII.
>
> >   1. Add a trace_key for sending machine-readable progress output to a
> >      descriptor or file. E.g., via setting GIT_TRACE_PROGRESS=2 in the
> >      environment.
> >
> >   2. Teach the trace code to open a command for piping, so that you
> >      could do something like GIT_TRACE_PROGRESS='|mygauge'.
> >
> > That would make your use case work, and I think many other use cases
> > would benefit from both of those features independently.
>
> Yup, that's all sensible, and would be great both for this and other
> stuff if we wanted true extensibility for this sort of thing.
>
> I'll just add that a 3rd thing that would also make sense would be to
> add a feature to configure the value of these GIT_TRACE_*=* variables
> via the .gitconfig, that's been suggested before (too lazy to dig up a
> ML archive reference), and would make this as easy to configure as
> Sebastian's suggestion.

I'm less concern about prettiness than showing numbers that are hard
to interpret. My other option was just showing ".", "..", "..."
sequence, which achieves the same thing.

I'm not opposed to having core.progressor or something like that. We
already have core.pager and this new config serves more or less the
same purpose. But I don't think I'll put time into implementing it.

-- 
Duy