threads / patch / 23963
Subject: [PATCH 0/3] Improve git-am error messages
Ramkumar RamachandraHi,
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(-)
Ramkumar Ramachandra[PATCH 1/3] Set cmdline globally, not in stop_here_user_resolve
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
Johannes SixtRe: [PATCH 1/3] Set cmdline globally, not in stop_here_user_resolve
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
Ramkumar RamachandraRe: [PATCH 1/3] Set cmdline globally, not in stop_here_user_resolve
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
Ramkumar Ramachandra[PATCH 2/3] Display some help text when patch is empty
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
Ramkumar Ramachandra[PATCH 3/3] Remove stray error message from sed
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
Junio C HamanoRe: [PATCH 3/3] Remove stray error message from sed
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