Re: CR codes from git commands

Daniel Barkalow [off-list ref] writes:

The terminal type, at least in my version of Emacs, is "dumb", which ought 
to be sufficient to tell git that a pager isn't going to be useful is most 
cases (might be worthwhile to keep "git log" from eating all your memory, 
though), and that using CR to rewrite lines isn't going to work.

I think we pay attention to "dumb" when deciding if pager is useful and if
we can do color, but I do not think we check anything beyond "is it a tty"
when deciding to show progress or not.  The only thing we do differently
for "dumb" terminal is if we use ANSI clear-to-eol escape sequence or fill
with a run of SPs to overwrite trailing part of a line, and we assume even
dumb terminals know how to do a carriage-return.

2009/1/22 Brent Goodrick [off-list ref]:
The environment I'm running git under is the Shell mode inside GNU
Emacs. I can't tell you what type of terminal it is, because I believe
that is defined deep in the guts of Emacs. Having read your reply
above, I'm now wondering whether this is an Emacs issue versus a git
issue. If it is an Emacs issue, then I am truly embarrassed for having
wasted everyones time with it.

2009/1/22 Junio C Hamano [off-list ref]:

I think we pay attention to "dumb" when deciding if pager is useful and if
we can do color, but I do not think we check anything beyond "is it a tty"
when deciding to show progress or not.  The only thing we do differently
for "dumb" terminal is if we use ANSI clear-to-eol escape sequence or fill
with a run of SPs to overwrite trailing part of a line, and we assume even
dumb terminals know how to do a carriage-return.

I think this earlier discussion is probably relevant... I'm guessing
though, $EDITOR is set correctly here 8-)

2008/12/17 Junio C Hamano [off-list ref]:

Any semi-good emacs users (let alone hackers) export PAGER=cat to be used
in compilation mode (and possibly shell mode), so this is not a problem in
practice.

I have something like this in my .emacs:

   (setenv "PAGER" "cat")

I suspect (I am just a user not a hacker) this will have bad interaction
with emacs terminal emulation mode, but I do not use the mode, so it is
enough for me.

Mike

Mike Ralphson writes:
 > >2009/1/22 Brent Goodrick [off-list ref]:
 > > The environment I'm running git under is the Shell mode inside GNU
 > > Emacs. I can't tell you what type of terminal it is, because I believe
 > > that is defined deep in the guts of Emacs. Having read your reply
 > > above, I'm now wondering whether this is an Emacs issue versus a git
 > > issue. If it is an Emacs issue, then I am truly embarrassed for having
 > > wasted everyones time with it.
 > 
 > 2009/1/22 Junio C Hamano [off-list ref]:
 > > I think we pay attention to "dumb" when deciding if pager is useful and if
 > > we can do color, but I do not think we check anything beyond "is it a tty"
 > > when deciding to show progress or not.  The only thing we do differently
 > > for "dumb" terminal is if we use ANSI clear-to-eol escape sequence or fill
 > > with a run of SPs to overwrite trailing part of a line, and we assume even
 > > dumb terminals know how to do a carriage-return.
 > 
 > I think this earlier discussion is probably relevant... I'm guessing
 > though, $EDITOR is set correctly here 8-)

I do have EDITOR set to a home-built version of gnuclient, and git
talks to Emacs by way of that gnuclient just fine when I'm not using the
-m "commit_message" git-commit option.

 > 
 > 2008/12/17 Junio C Hamano [off-list ref]:
 > > Any semi-good emacs users (let alone hackers) export PAGER=cat to be used
 > > in compilation mode (and possibly shell mode), so this is not a problem in
 > > practice.
 > >
 > > I have something like this in my .emacs:
 > >
 > >    (setenv "PAGER" "cat")
 > >
 > > I suspect (I am just a user not a hacker) this will have bad interaction
 > > with emacs terminal emulation mode, but I do not use the mode, so it is
 > > enough for me.

I have PAGER set to "cat" in the environment before I run Emacs for
the same reason.

Unfortunately, this morning when I rebooted and reloaded from scratch,
I am now unable to reproduce the CR codes output from "git pull" no
matter what I do. I even tried the older git installed on Debian Linux
"testing", and tried unsetting PAGER and GIT_PAGER, and saw the pager
prompts and the terminal escape sequence output as I expected to
(which is not the issue here).  I can't expect anyone else to help me
debug this problem further if I can't even reproduce it
anymore. Frustrating.

I do have automatic updates turned on, so perhaps something changed in
the termcap or how terminal I/O is being done outside of git in my
system.  Emacs would not have changed since I build Emacs from top of
trunk CVS, and it only uses local Elisp packages AFAIK.

I don't suppose git has any logic that emits the progress messages
based upon some estimate of amount of work it has to do, or has done,
does it?

Thanks,
Brent

P.S., for your reference, below is my evaluation script that
previously showed the CR code from git pull output. I even increased
the number of files added to the second repo up to 50 to see if the
quantity of files being pulled had any effect on the progress messages
output, but that didn't seem to have any effect. If anyone sees
anything bone-headed there, I'm all ears:

--- cut below this line --- 

#!/bin/sh
# -*-mode: Shell-script; indent-tabs-mode: nil; -*-

# I could have simply used "set -x" here but then I wouldn't see the
# redirection syntax like ">file1", so instead use a PrintRun
# function:
PrintRun ()
{
    echo "COMMAND: $*"
    eval "$*; exitcode=\$?"
    if [ $exitcode != 0 ]
    then
        echo "ERROR: Command failed: $*"
        exit 1
    fi
}

git_term_redirect=""
if [ "$USE_GIT_TERM_REDIRECT" = 1 ]
then
    git_term_redirect=" 2>&1 | cat"
    echo "Note: using git redirect on some git git commands: \"$git_term_redirect\""
fi

if [ "$USE_LOCALLY_BUILT_GIT" = 1 ]
then
    git_bin_dir="$HOME/git_from_source/install/bin"
    if [ -d "$git_bin_dir" ]
    then
        PATH="$HOME/git_from_source/install/bin:$PATH"; export PATH
    fi
fi

if [ "$SKIP_PAGER_HACK" = 1 ]
then
    unset PAGER
    echo "Note: setting PAGER to $PAGER"
else
    echo "Note: unsetting PAGER"
    PAGER=cat; export PAGER
fi

# Print out the git version as a double check on the above logic:
PrintRun git --version
# Clear out the scratch areas:
PrintRun rm -rf /tmp/git_area1
PrintRun rm -rf /tmp/git_area2
# Populate the initial area:
PrintRun mkdir -p /tmp/git_area1
PrintRun cd /tmp/git_area1
PrintRun git init
PrintRun "echo a new file 1 >file1"
PrintRun "echo a new file 2 >file2"
PrintRun git add file1
PrintRun git add file2
PrintRun git status
PrintRun "git commit -m \"first commit in git_area1\""
PrintRun find .
# Clone from the first area into a second area and add files there:
PrintRun rm -rf /tmp/git_area2
PrintRun cd /tmp
PrintRun git clone /tmp/git_area1 git_area2

PrintRun cd /tmp/git_area2
PrintRun find .
i=1
while [ $i -le 50 ]
do
    file="file_$i"
    echo "file==\"${file}\""
    PrintRun "echo a new file >$file"
    PrintRun git add $file
    PrintRun git status
    PrintRun "git commit -m \"committing new file $file but in git_area2\""
    #    PrintRun "git status; true" # true means don't fail inside PrintRun
    i=`expr $i + 1`
done

# Now attempt to pull the second repo changes back into into the first repo with a "git pull" operation:

PrintRun cd /tmp/git_area1
PrintRun "git status; true" # true means don't fail inside PrintRun
PrintRun "git diff; true" # true means don't fail inside PrintRun
if [ "$INJECT_TERM" != "" ]
then
    echo "Note: Exporting environment variable: TERM=\"$INJECT_TERM\""
    TERM="$INJECT_TERM"; export TERM
fi
PrintRun "git pull /tmp/git_area2 master $git_term_redirect"
# if [ "$STOP_AFTER_FIRST_GIT_PULL" = 1 ]
# then
#     echo "Note: Stopping after first git pull"
#     env | grep -i term
#     exit 0
# fi
PrintRun "git status; true" # true means don't fail inside PrintRun
PrintRun cat file_3
PrintRun "echo conflict1 >>file_3"
PrintRun git add file_3
PrintRun git status
PrintRun "git commit -m \"conflict1 added in git_area1\""

PrintRun cd /tmp/git_area2
PrintRun "echo conflict2 >>file_3"
PrintRun git add file_3
PrintRun git status
PrintRun "git commit -m \"conflict2 added in git_area2\""


PrintRun cd /tmp/git_area1
PrintRun "git status; true" # true means don't fail inside PrintRun
PrintRun "git diff; true" # true means don't fail inside PrintRun
# This git pull should show the conflict:
PrintRun "git pull /tmp/git_area2 master $git_term_redirect"
PrintRun cat file_3
PrintRun "echo conflict resolved > file_3"
# Running git commit now will fail:
### PrintRun "git commit -m \"conflict resolved\""
# Running git add on the file I just "resolved" by editing it directly above
PrintRun git add file_3
PrintRun "git status; true" # true means don't fail inside PrintRun
PrintRun "git commit -m \"conflict resolved\""
PrintRun "git log"

--- cut above this line --- 

2009/1/22 Brent Goodrick [off-list ref]:

Mike Ralphson writes:
 > I think this earlier discussion is probably relevant... I'm guessing
 > though, $EDITOR is set correctly here 8-)

I do have EDITOR set to a home-built version of gnuclient...

Sorry, I was being too subtle. My $EDITOR is set to vim, as god intended. 8-)

Mike

On Thu, 22 Jan 2009, Brent Goodrick wrote:

Mike Ralphson writes:
 > >2009/1/22 Brent Goodrick [off-list ref]:
 > > The environment I'm running git under is the Shell mode inside GNU
 > > Emacs. I can't tell you what type of terminal it is, because I believe
 > > that is defined deep in the guts of Emacs. Having read your reply
 > > above, I'm now wondering whether this is an Emacs issue versus a git
 > > issue. If it is an Emacs issue, then I am truly embarrassed for having
 > > wasted everyones time with it.
 > 
 > 2009/1/22 Junio C Hamano [off-list ref]:
 > > I think we pay attention to "dumb" when deciding if pager is useful and if
 > > we can do color, but I do not think we check anything beyond "is it a tty"
 > > when deciding to show progress or not.  The only thing we do differently
 > > for "dumb" terminal is if we use ANSI clear-to-eol escape sequence or fill
 > > with a run of SPs to overwrite trailing part of a line, and we assume even
 > > dumb terminals know how to do a carriage-return.
 > 
 > I think this earlier discussion is probably relevant... I'm guessing
 > though, $EDITOR is set correctly here 8-)

I do have EDITOR set to a home-built version of gnuclient, and git
talks to Emacs by way of that gnuclient just fine when I'm not using the
-m "commit_message" git-commit option.

 > 
 > 2008/12/17 Junio C Hamano [off-list ref]:
 > > Any semi-good emacs users (let alone hackers) export PAGER=cat to be used
 > > in compilation mode (and possibly shell mode), so this is not a problem in
 > > practice.
 > >
 > > I have something like this in my .emacs:
 > >
 > >    (setenv "PAGER" "cat")
 > >
 > > I suspect (I am just a user not a hacker) this will have bad interaction
 > > with emacs terminal emulation mode, but I do not use the mode, so it is
 > > enough for me.

I have PAGER set to "cat" in the environment before I run Emacs for
the same reason.

Unfortunately, this morning when I rebooted and reloaded from scratch,
I am now unable to reproduce the CR codes output from "git pull" no
matter what I do. I even tried the older git installed on Debian Linux
"testing", and tried unsetting PAGER and GIT_PAGER, and saw the pager
prompts and the terminal escape sequence output as I expected to
(which is not the issue here).  I can't expect anyone else to help me
debug this problem further if I can't even reproduce it
anymore. Frustrating.

I do have automatic updates turned on, so perhaps something changed in
the termcap or how terminal I/O is being done outside of git in my
system.  Emacs would not have changed since I build Emacs from top of
trunk CVS, and it only uses local Elisp packages AFAIK.

I don't suppose git has any logic that emits the progress messages
based upon some estimate of amount of work it has to do, or has done,
does it?

It does have logic to only emit progress messages at a reasonable rate 
(otherwise, you might be waiting for the progress messages to be printed 
instead of just waiting for the data to arrive). So it's possible that you 
now have things going fast enough that it only needs to print one message. 
It can also estimate that something hasn't taken long enough for the user 
to get impatient yet, and therefore not show progress at all (so the 
output won't be littered with progress output for every operation that 
could have taken a long time for some data, but didn't for this data).

In any case, it's all done in progress.c, so it should be easy enough to 
make changes to if you can come up with something better to do with 
progress messages and some way to determine when it should be done.

	-Daniel
*This .sig left intentionally blank*

Hi,

On Thu, 22 Jan 2009, Mike Ralphson wrote:

My $EDITOR is set to vim, as god intended. 8-)

Sorry, that is not true: from

http://www.biblegateway.com/passage/?book_id=50&chapter=1&verse=1&version=31&context=verse

we know that in the beginning was the Word.

Ciao,
Dscho

Hi,

On Thu, 22 Jan 2009, Daniel Barkalow wrote:

In any case, it's all done in progress.c, so it should be easy enough to 
make changes to if you can come up with something better to do with 
progress messages and some way to determine when it should be done.

Maybe "git --no-progress <program>" would be a sensible user interface?

Ciao,
Dscho

Johannes Schindelin writes:
 > Hi,
 > 
 > On Thu, 22 Jan 2009, Daniel Barkalow wrote:
 > 
 > > In any case, it's all done in progress.c, so it should be easy enough to 
 > > make changes to if you can come up with something better to do with 
 > > progress messages and some way to determine when it should be done.
 > 
 > Maybe "git --no-progress <program>" would be a sensible user
 > interface?

Thanks. I now see the \r reference inside the "display" file-static
function inside progress.c.

However, I propose to add two options, the first being, IMO, the
minimal one to implement, and the second being "nice-to-have":

 - Bare minimum: Add a new --no-cr option (e.g., "git --no-cr
   <program>") that would prevent any git code (inside progress.c or
   elsewhere) from emitting a CR code from stdout or stderr.  This has
   the effect of allowing progress messages, but not asking too much
   of terminals-that-are-not-really-terminals such as the GNU Emacs
   shell mode.

 - Nice-to-have: Add a "git --no-progress" message that would never
   show progress at all (e.g., perhaps by not installing a signal
   handler inside progress.c such that no messages would not be
   emitted at all.

Both options are intended to be independent of each other.

And for both options, I would like there to be a config option to
allow the user to enable said behavior globally across all git
operations covered by that config file.

I might be willing to take a swipe at this myself and submit a patch,
provided I receive adequate noobie hand-holding (or hand-slapping) on
patch submission and test case development.

bg

Hi,

On Fri, 23 Jan 2009, Brent Goodrick wrote:

 - Bare minimum: Add a new --no-cr option

I do not see any value of this over "--progress | tr '\r' '\n'".  (The 
--progress option being the natural counterpart to --no-progress, 
_forcing_ the display of the progress.)

And I disagree that --no-progress would be hard to implement.  Just have a 
look at 7d1864c(Introduce is_bare_repository() and core.bare configuration 
variable).

Basically, you'll have to

- introduce a global variable to both environment.c and cache.h,

- set it to -1 by default,

- handle a "--progress" and "--no-progress" option in git.c, setting the 
  global variable git_show_progress to 1 or 0, respectively,

- teach start_progress_delay() to return NULL if git_show_progress == 0,

- modify all users of start_progress*() to respect git_show_progress == 1,
  which probably means to look for "isatty" in builtin-pack-objects.c and 
  builtin-unpack-objects.c

- add documentation to Documentation/git.txt what --progress and 
  --no-progress do,

- add a simple test script to t/ (maybe t/t0005-progress.sh) that tests 
  that --progress works -- maybe you find a clever way to test 
  --no-progress, too, but that would be harder, as the progress is turned 
  off by default for the scripts anyway...)

Hth,
Dscho

Junio C Hamano writes:
 > I do not think so.  --no-progress should imply --no-cr ;-)
 > 
 > I do not think it makes much sense to pollute your non-terminal with 100
 > lines of 1%,2%,3%,...100% if it cannot sensibly do carriage-returns.  It
 > may be another knob to tweak, but it's a kind of thing you implement
 > because you could, not because it makes sense.  I would be mildly against
 > no-cr.

Good point. I'll drop the --no-cr as redundant.

Johannes Schindelin writes:
 > Hi,
 > 
 > On Fri, 23 Jan 2009, Brent Goodrick wrote:
 > 
 > >  - Bare minimum: Add a new --no-cr option
 > 
 > I do not see any value of this over "--progress | tr '\r' '\n'".  (The 
 > --progress option being the natural counterpart to --no-progress, 
 > _forcing_ the display of the progress.)

Agreed. Both --progress and --no-progress are the only options to be
implemented for this.  

 > Just have a 
 > look at 7d1864c(Introduce is_bare_repository() and core.bare configuration 
 > variable).

Note that I'm coming from a CVS and Perforce user background but am
still new to git usage. How do I "take a look" at "7d1864c"?

I will take a closer look at the list of things you explained in your
"Basically, you'll have to" list.

While I'm at it, what is the standard procedure for submitting git
patches for review once I've cooked up and validated it on my end? I'm
guessing posting the patch into this mailing list is part of the
answer to that question.

Thanks,
Brent

Hi,

On Sat, 24 Jan 2009, Brent Goodrick wrote:

Note that I'm coming from a CVS and Perforce user background but am 
still new to git usage. How do I "take a look" at "7d1864c"?

Do this in a checkout of git.git:

$ git show 7d1864c

Alternatively, you can follow this URL:

	http://repo.or.cz/w/git.git?a=commitdiff;h=7d1864c

Ciao,
Dscho

On Saturday 24 January 2009, Brent Goodrick [off-list ref] wrote 
about 'Re: CR codes from git commands':

While I'm at it, what is the standard procedure for submitting git
patches for review once I've cooked up and validated it on my end? I'm
guessing posting the patch into this mailing list is part of the
answer to that question.

If you've got a patch, I assume you've got a checkout.  Look in 
Documentation/SubmittingPatches.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

Boyd Stephen Smith Jr. writes:
 > On Saturday 24 January 2009, Brent Goodrick [off-list ref] wrote 
 > about 'Re: CR codes from git commands':
 > >While I'm at it, what is the standard procedure for submitting git
 > >patches for review once I've cooked up and validated it on my end? I'm
 > >guessing posting the patch into this mailing list is part of the
 > >answer to that question.
 > 
 > If you've got a patch, I assume you've got a checkout.  Look in 
 > Documentation/SubmittingPatches.

Thanks I see that now. No, I don't have a patch yet, was struggling to
find that basic info that really should be front and center somewhere
on the wiki (and also access to the wiki is very slow).

bg

Brent Goodrick [off-list ref] writes:

While I'm at it, what is the standard procedure for submitting git
patches for review once I've cooked up and validated it on my end? I'm
guessing posting the patch into this mailing list is part of the
answer to that question.

Yes, a guideline is in Documentation/SubmittingPatches for the initial
submission.  After that, the lifecycle of a patch submitted on the list
goes like this:

 (1) A patch is shown to the list participants.

 (2) People may like it, or may have issues with it, and responds with
     their comments describing problems, suggestions for improvements,
     etc.  People who are not interested in the topic may stay silent.

 (3) The original author responds with updated patch.  Sometimes people
     who commented on in step 2 may even send "here is how I would do this
     one; don't you think this is better?", and the original author may
     say "Yeah, let's use yours instead".

 (4) After steps 2 and 3 repeats zero or more times, the latest patch may
     become one that everyone likes, or at least nobody has trouble with
     inclusion.  The author sends such a patch saying "this is meant for
     inclusion based on discussion and refinements in these threads...".

 (5) The maintainer picks it up when it looks polished enough.

Your patch may appear in the periodical "What's cooking" or "What's in"
summary with zero iteration of steps 2 and 3 if it is obvious enough.

I act as just one of the list participant during steps 1-3.  I may stay
silent during this period but that only means the topic is not interesting
to me and nothing more.  It does not mean that the topic has no chance of
getting included.

I act as the maintainer for steps 4 and 5.  If you do not hear from me
after step 4, then I am either being lazy, busy, or sick, or the patch got
lost in the noise and I need a reminder.  Note that I may reject or ask
further refinement at step 4 to ensure overall quality throughout the
system even in areas I am not interested in and didn't say anything during
steps 1-3.

I'm nearing completion on the patch for the --progress and
--no-progress command-line options.  I am able to manually validate
the behavior, but am a bit stumped as to how to efficiently code up
the test script.  My manual test involves doing a git clone of the git
repository, which produces the volume of I/O sufficiently bulky to
trigger the progress message code.  But that bulk means that the test
case will take a long time to complete, hence making using a git clone
of the git code in the test case impractical.

Also, in order for the script to do its job, it will need to tell the
difference between a git run that has progress from one that does not.
 The first idea would be to simply use shell command redirection on
the git command itself, but that defeats the tty detection logic, so I
don't think that is an option either.

Does anyone have any recommendations here? If not, then I guess I will
have to forgo the test script and just submit the patch without it.

Thanks,
Brent

On Sun, Jan 25, 2009 at 1:19 AM, Boyd Stephen Smith Jr.
[off-list ref] wrote:

On Saturday 24 January 2009, Brent Goodrick [off-list ref] wrote
about 'Re: CR codes from git commands':

quoted

While I'm at it, what is the standard procedure for submitting git
patches for review once I've cooked up and validated it on my end? I'm
guessing posting the patch into this mailing list is part of the
answer to that question.

If you've got a patch, I assume you've got a checkout.  Look in
Documentation/SubmittingPatches.
--
Boyd Stephen Smith Jr.                     ,= ,-_-. =.
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-'
http://iguanasuicide.net/                      \_/