threads / discuss / 49397

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

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

## tl;dr

6 messages between Sep 21, 2018 and Sep 23, 2018.

replies: 5people: 4as markdown or json

Sebastian Gniazdowski· Sep 21, 2018, 13:34 UTC · lore
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
Jeff King· Sep 21, 2018, 22:24 UTC · re: Sebastian Gniazdowski · lore

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

On Fri, Sep 21, 2018 at 09:34:14AM -0400, Sebastian Gniazdowski wrote:
Show 16 quoted lines
> 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
Ævar Arnfjörð Bjarmason· Sep 21, 2018, 23:30 UTC · re: Jeff King · lore

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

On Fri, Sep 21 2018, Jeff King wrote:
Show 21 quoted lines
> 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.

Show 9 quoted lines
>   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.

Jeff King· Sep 21, 2018, 23:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Sat, Sep 22, 2018 at 01:30:36AM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 9 quoted lines
> >> 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).

Show 18 quoted lines
> >   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
Sebastian Gniazdowski· Sep 22, 2018, 18:16 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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. 
Show 5 quoted lines
> 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
Duy Nguyen· Sep 23, 2018, 13:06 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Sat, Sep 22, 2018 at 1:30 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 23 quoted lines
> 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

← back to recent threads