[PATCH 0/3] Improve git-am error messages

[PATCH 0/3] Improve git-am error messages

[Ramkumar Ramachandra]

Hi,

This patch series aims to improve some of the error messages in git-am
that I came across.

-- Ram

Ramkumar Ramachandra (3):
  Initialize cmdline globally, not in stop_here_user_resolve
  Display some help text when patch is empty
  Remove stray error message from sed when continuing without changes

 git-am.sh |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

[PATCH 1/3] Set cmdline globally, not in stop_here_user_resolve

[Ramkumar Ramachandra]

Set the $cmdline variable globally, so it can be used in other code
fragments as well. Also, instead of hardcoding the string "git am",
use the command line argument "$0".

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-am.sh |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 87ffae2..b779277 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -52,6 +52,16 @@ else
 	HAS_HEAD=
 fi
 
+cmdline="$0"
+if test '' != "$interactive"
+then
+    cmdline="$cmdline -i"
+fi
+if test '' != "$threeway"
+then
+    cmdline="$cmdline -3"
+fi
+
 sq () {
 	git rev-parse --sq-quote "$@"
 }
@@ -66,15 +76,6 @@ stop_here_user_resolve () {
 	    printf '%s\n' "$resolvemsg"
 	    stop_here $1
     fi
-    cmdline="git am"
-    if test '' != "$interactive"
-    then
-        cmdline="$cmdline -i"
-    fi
-    if test '' != "$threeway"
-    then
-        cmdline="$cmdline -3"
-    fi
     echo "When you have resolved this problem run \"$cmdline --resolved\"."
     echo "If you would prefer to skip this patch, instead run \"$cmdline --skip\"."
     echo "To restore the original branch and stop patching run \"$cmdline --abort\"."
-- 
1.7.1

Re: [PATCH 1/3] Set cmdline globally, not in stop_here_user_resolve

[Johannes Sixt]

Am 6/1/2010 11:20, schrieb Ramkumar Ramachandra:
> Set the $cmdline variable globally, so it can be used in other code
> fragments as well. Also, instead of hardcoding the string "git am",
> use the command line argument "$0".

Did you test this? "$0" prints the full path to git-am. This does not
count as user-friendly in my book!

-- Hannes

Re: [PATCH 1/3] Set cmdline globally, not in stop_here_user_resolve

[Ramkumar Ramachandra]

Hi,

> Did you test this? "$0" prints the full path to git-am. This does not
> count as user-friendly in my book!

Doesn't it print out exactly the way you invoke the application ie.
either "./git-am.sh" or "git am"? Then again, when git-am is invoked
internally by git, it possibly uses the full path instead of relying
on $PATH, and this is quite ugly. I noticed that all the other other
shell scripts in git.git also hardcode the string, and there must be a
good reason for this. Therefore I think this particular change should
be dropped.

Thanks for pointing this out.

-- Ram

[PATCH 2/3] Display some help text when patch is empty

[Ramkumar Ramachandra]

When a patch is found to be empty, prompt the user to use either
--skip or --abort.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-am.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index b779277..04f02a8 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -592,6 +592,8 @@ do
 
 		test -s "$dotest/patch" || {
 			echo "Patch is empty.  Was it split wrong?"
+			echo "If you would prefer to skip this patch, instead run \"$cmdline --skip\"."
+			echo "To restore the original branch and stop patching run \"$cmdline --abort\"."
 			stop_here $this
 		}
 		rm -f "$dotest/original-commit"
-- 
1.7.1

[PATCH 3/3] Remove stray error message from sed

[Ramkumar Ramachandra]

When --continue is invoked without any changes, the following stray
error message appears- sed: can't read $dotest/final-commit: No such
file or directory. Remove this by making sure that the file actually
exists.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-am.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 04f02a8..e61f47a 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -693,7 +693,7 @@ do
 	else
 	    action=yes
 	fi
-	FIRSTLINE=$(sed 1q "$dotest/final-commit")
+	test -e "$dotest/final-commit" && FIRSTLINE=$(sed 1q "$dotest/final-commit")
 
 	if test $action = skip
 	then
-- 
1.7.1

Re: [PATCH 3/3] Remove stray error message from sed

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> When --continue is invoked without any changes, the following stray
> error message appears- sed: can't read $dotest/final-commit: No such
> file or directory. Remove this by making sure that the file actually
> exists.

>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  git-am.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 04f02a8..e61f47a 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -693,7 +693,7 @@ do
>  	else
>  	    action=yes
>  	fi
> -	FIRSTLINE=$(sed 1q "$dotest/final-commit")
> +	test -e "$dotest/final-commit" && FIRSTLINE=$(sed 1q "$dotest/final-commit")

This will let the command follow the same codepath as before but the
change in behaviour is that it does not reset FIRSTLINE to an empty
string.  Does this difference affect what the user sees after this part of
the code?

I would generally prefer to use "test -f" not "-e" (simply because it came
later and I am an old-timer), but if you want to use something newer than
"test -f", it might be worth using "test -r" instead, as readability (not
existence) is what you are after anyway.

>  
>  	if test $action = skip
>  	then
> -- 
> 1.7.1