Re: [PATCH 3/2] test: factor out helper function test_must_contain

2015-11-21 Thread Johannes Sixt

Am 20.11.2015 um 21:50 schrieb René Scharfe:

Extract a helper function for searching for a pattern in a file and
printing the whole file if the pattern is not found.  It is useful
when starting tests with --verbose for debugging purposes.



+# Check if a file contains an expected pattern.
+test_must_contain () {
+   if grep "$1" "$2"
+   then
+   return 0
+   else
+   echo "didn't find /$1/ in '$2', it contains:"
+   cat "$2"
+   return 1
+   fi
+}


There is already test_i18n_grep. Should it be folded into this function? 
Wouldn't we also want to have a function test_must_not_contain?


IMHO, we should not increase the number of functions that give a bonus 
only when there is a test case failure. They do not scale well: There is 
a permanent mental burden on every reviewer to watch out that they are 
used in new tests. But without those functions, the burden is on the one 
person investigating a test case failure, who has to live without the 
debugging support.


-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/2] test: factor out helper function test_must_contain

2015-11-21 Thread René Scharfe
Am 21.11.2015 um 09:11 schrieb Johannes Sixt:
> Am 20.11.2015 um 21:50 schrieb René Scharfe:
>> Extract a helper function for searching for a pattern in a file and
>> printing the whole file if the pattern is not found.  It is useful
>> when starting tests with --verbose for debugging purposes.
> 
>> +# Check if a file contains an expected pattern.
>> +test_must_contain () {
>> +if grep "$1" "$2"
>> +then
>> +return 0
>> +else
>> +echo "didn't find /$1/ in '$2', it contains:"
>> +cat "$2"
>> +return 1
>> +fi
>> +}
> 
> There is already test_i18n_grep. Should it be folded into this function? 

That's a good point.  But how?  test_i18ngrep can also work as a
filter and pass on options, so we'd need to parse all parameters and
redirect stdin to a temporary file unless a file was specified.  Or
we could be sloppy and just check if the last parameter is a file and
if yes then spew it out.

> Wouldn't we also want to have a function test_must_not_contain?

I doubt it.  In such a function grep would display the lines that match
unexpectedly already, so showing the whole file after that won't add
much more of interest.

> IMHO, we should not increase the number of functions that give a bonus 
> only when there is a test case failure. They do not scale well: There is 
> a permanent mental burden on every reviewer to watch out that they are 
> used in new tests. But without those functions, the burden is on the one 
> person investigating a test case failure, who has to live without the 
> debugging support.

test_must_contain doesn't have to be used everywhere, only in cases
where a file is shown and grepped.  I agree that letting an existing
function do that job (or deciding that the job is not worth doing) is
preferable.

Here's how I imagine the sloppy add-on to test_i18ncmp to look:

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 16c4d7b..db64600 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -985,13 +985,28 @@ test_i18ncmp () {
 test_i18ngrep () {
if test -n "$GETTEXT_POISON"
then
: # pretend success
elif test "x!" = "x$1"
then
shift
! grep "$@"
else
grep "$@"
+
+   rc=$?
+   if test $rc != 0
+   then
+   while test $# -gt 1
+   do
+   shift
+   done
+   if test -f "$1"
+   then
+   echo "Expected pattern not found, content is:"
+   cat "$1"
+   fi
+   return $rc
+   fi
fi
 }
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2] git-p4: allow submission from detached head

2015-11-21 Thread Luke Diamand
I'm resending my reroll of my earlier patch to teach git-p4
about detached heads.

It uses Junio's suggestion of calling "git symbolic-ref"
to determine if we're on a detached head, rather than
parsing text strings.

Luke Diamand (3):
  git-p4: add failing test for submit from detached head
  git-p4: add option to system() to return subshell status
  git-p4: work with a detached head

 git-p4.py   | 29 -
 t/t9800-git-p4-basic.sh | 16 
 2 files changed, 36 insertions(+), 9 deletions(-)

-- 
2.6.0.rc3.238.gc07a1e8

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/3] git-p4: add failing test for submit from detached head

2015-11-21 Thread Luke Diamand
git-p4 can't submit from a detached head. This test case
demonstrates the problem.

Signed-off-by: Luke Diamand 
---
 t/t9800-git-p4-basic.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 90d41ed..114b19f 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -241,6 +241,22 @@ test_expect_success 'unresolvable host in P4PORT should 
display error' '
)
 '
 
+test_expect_failure 'submit from detached head' '
+   test_when_finished cleanup_git &&
+   git p4 clone --dest="$git" //depot &&
+   (
+   cd "$git" &&
+   git checkout p4/master &&
+   >detached_head_test &&
+   git add detached_head_test &&
+   git commit -m "add detached_head" &&
+   git config git-p4.skipSubmitEdit true &&
+   git p4 submit &&
+   git p4 rebase &&
+   git log p4/master | grep detached_head
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
2.6.0.rc3.238.gc07a1e8

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/3] git-p4: work with a detached head

2015-11-21 Thread Luke Diamand
When submitting, git-p4 finds the current branch in
order to know if it is allowed to submit (configuration
"git-p4.allowSubmit").

On a detached head, detecting the branch would fail, and
git-p4 would report a cryptic error.

This change teaches git-p4 to recognise a detached head and
submit successfully.

Signed-off-by: Luke Diamand 
---
 git-p4.py   | 23 ---
 t/t9800-git-p4-basic.sh |  2 +-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 9d55f9c..0cfc866 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -544,7 +544,12 @@ def p4Where(depotPath):
 return clientPath
 
 def currentGitBranch():
-return read_pipe("git name-rev HEAD").split(" ")[1].strip()
+retcode = system(["git", "symbolic-ref", "-q", "HEAD"], ignore_error=True)
+if retcode != 0:
+# on a detached head
+return None
+else:
+return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
 
 def isValidGitDir(path):
 if (os.path.exists(path + "/HEAD")
@@ -1653,8 +1658,6 @@ class P4Submit(Command, P4UserMap):
 def run(self, args):
 if len(args) == 0:
 self.master = currentGitBranch()
-if len(self.master) == 0 or not gitBranchExists("refs/heads/%s" % 
self.master):
-die("Detecting current git branch failed!")
 elif len(args) == 1:
 self.master = args[0]
 if not branchExists(self.master):
@@ -1662,9 +1665,10 @@ class P4Submit(Command, P4UserMap):
 else:
 return False
 
-allowSubmit = gitConfig("git-p4.allowSubmit")
-if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
-die("%s is not in git-p4.allowSubmit" % self.master)
+if self.master:
+allowSubmit = gitConfig("git-p4.allowSubmit")
+if len(allowSubmit) > 0 and not self.master in 
allowSubmit.split(","):
+die("%s is not in git-p4.allowSubmit" % self.master)
 
 [upstream, settings] = findUpstreamBranchPoint()
 self.depotPath = settings['depot-paths'][0]
@@ -1732,7 +1736,12 @@ class P4Submit(Command, P4UserMap):
 self.check()
 
 commits = []
-for line in read_pipe_lines(["git", "rev-list", "--no-merges", 
"%s..%s" % (self.origin, self.master)]):
+if self.master:
+commitish = self.master
+else:
+commitish = 'HEAD'
+
+for line in read_pipe_lines(["git", "rev-list", "--no-merges", 
"%s..%s" % (self.origin, commitish)]):
 commits.append(line.strip())
 commits.reverse()
 
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 114b19f..0730f18 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -241,7 +241,7 @@ test_expect_success 'unresolvable host in P4PORT should 
display error' '
)
 '
 
-test_expect_failure 'submit from detached head' '
+test_expect_success 'submit from detached head' '
test_when_finished cleanup_git &&
git p4 clone --dest="$git" //depot &&
(
-- 
2.6.0.rc3.238.gc07a1e8

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/3] git-p4: add option to system() to return subshell status

2015-11-21 Thread Luke Diamand
Add an optional parameter ignore_error to the git-p4 system()
function. If used, it will return the subshell exit status
rather than throwing an exception.

Signed-off-by: Luke Diamand 
---
 git-p4.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 0093fa3..9d55f9c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -192,14 +192,16 @@ def p4_has_move_command():
 # assume it failed because @... was invalid changelist
 return True
 
-def system(cmd):
+def system(cmd, ignore_error=False):
 expand = isinstance(cmd,basestring)
 if verbose:
 sys.stderr.write("executing %s\n" % str(cmd))
 retcode = subprocess.call(cmd, shell=expand)
-if retcode:
+if retcode and not ignore_error:
 raise CalledProcessError(retcode, cmd)
 
+return retcode
+
 def p4_system(cmd):
 """Specifically invoke p4 as the system command. """
 real_cmd = p4_build_cmd(cmd)
-- 
2.6.0.rc3.238.gc07a1e8

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] bash prompt: test dirty index and worktree while on an orphan branch

2015-11-21 Thread SZEDER Gábor
There is only a single test exercising the dirty state indicator on an
orphan branch, and in that test neither the index nor the worktree are
dirty.

Add two failing tests to check the dirty state indicator while either
the index is dirty or while both the index and the worktree are dirty
on an orphan branch, and to show that the dirtiness of the index is
not displayed in these cases (the forth combination, i.e. clean index
and dirty worktree are impossible on an orphan branch).  Update the
existing dirty state indicator on clean orphan branch test to match
the style of the two new tests, most importantly to use 'git checkout
--orphan' instead of cd-ing into a repository that just happens to be
empty and clean.

Signed-off-by: SZEDER Gábor 
---
 t/t9903-bash-prompt.sh | 31 ---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 6b68777b98..2c9d1f928a 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -273,11 +273,36 @@ test_expect_success 'prompt - dirty status indicator - 
dirty index and worktree'
test_cmp expected "$actual"
 '
 
-test_expect_success 'prompt - dirty status indicator - before root commit' '
-   printf " (master #)" >expected &&
+test_expect_success 'prompt - dirty status indicator - orphan branch - clean' '
+   printf " (orphan #)" >expected &&
+   test_when_finished "git checkout master" &&
+   git checkout --orphan orphan &&
+   git reset --hard &&
+   (
+   GIT_PS1_SHOWDIRTYSTATE=y &&
+   __git_ps1 >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_failure 'prompt - dirty status indicator - orphan branch - dirty 
index' '
+   printf " (orphan +)" >expected &&
+   test_when_finished "git checkout master" &&
+   git checkout --orphan orphan &&
+   (
+   GIT_PS1_SHOWDIRTYSTATE=y &&
+   __git_ps1 >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_failure 'prompt - dirty status indicator - orphan branch - dirty 
index and worktree' '
+   printf " (orphan *+)" >expected &&
+   test_when_finished "git checkout master" &&
+   git checkout --orphan orphan &&
+   >file &&
(
GIT_PS1_SHOWDIRTYSTATE=y &&
-   cd otherrepo &&
__git_ps1 >"$actual"
) &&
test_cmp expected "$actual"
-- 
2.6.3.402.geb6a0f7

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/3] bash prompt: remove a redundant command line option

2015-11-21 Thread SZEDER Gábor
To get the dirty state indicator __git_ps1() runs 'git diff' with
'--quiet --exit-code' options.  '--quiet' already implies
'--exit-code', so the latter is unnecessary and can be removed.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-prompt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 07b52bedf1..7a95fbdcfd 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -476,7 +476,7 @@ __git_ps1 ()
if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&
   [ "$(git config --bool bash.showDirtyState)" != "false" ]
then
-   git diff --no-ext-diff --quiet --exit-code || w="*"
+   git diff --no-ext-diff --quiet || w="*"
if [ -n "$short_sha" ]; then
git diff-index --cached --quiet HEAD -- || i="+"
else
-- 
2.6.3.402.geb6a0f7

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] bash prompt: indicate dirty index even on orphan branches

2015-11-21 Thread SZEDER Gábor
__git_ps1() doesn't indicate dirty index while on an orphan branch.

To check the dirtiness of the index, __git_ps1() runs 'git diff-index
--cached ... HEAD', which doesn't work on an orphan branch,
because HEAD doesn't point to a valid commit.

Run 'git diff ... --cached' instead, as it does the right thing both
on valid and invalid HEAD, i.e. compares the index to the existing
HEAD in the former case and to the empty tree in the latter.  This
fixes the two failing tests added in the first commit of this series.

The dirtiness of the worktree is already checked with 'git diff' and
is displayed correctly even on an orphan branch.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-prompt.sh | 5 ++---
 t/t9903-bash-prompt.sh   | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 7a95fbdcfd..64219e631a 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -477,9 +477,8 @@ __git_ps1 ()
   [ "$(git config --bool bash.showDirtyState)" != "false" ]
then
git diff --no-ext-diff --quiet || w="*"
-   if [ -n "$short_sha" ]; then
-   git diff-index --cached --quiet HEAD -- || i="+"
-   else
+   git diff --no-ext-diff --cached --quiet || i="+"
+   if [ -z "$short_sha" ] && [ -z "$i" ]; then
i="#"
fi
fi
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 2c9d1f928a..af82049f82 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -285,7 +285,7 @@ test_expect_success 'prompt - dirty status indicator - 
orphan branch - clean' '
test_cmp expected "$actual"
 '
 
-test_expect_failure 'prompt - dirty status indicator - orphan branch - dirty 
index' '
+test_expect_success 'prompt - dirty status indicator - orphan branch - dirty 
index' '
printf " (orphan +)" >expected &&
test_when_finished "git checkout master" &&
git checkout --orphan orphan &&
@@ -296,7 +296,7 @@ test_expect_failure 'prompt - dirty status indicator - 
orphan branch - dirty ind
test_cmp expected "$actual"
 '
 
-test_expect_failure 'prompt - dirty status indicator - orphan branch - dirty 
index and worktree' '
+test_expect_success 'prompt - dirty status indicator - orphan branch - dirty 
index and worktree' '
printf " (orphan *+)" >expected &&
test_when_finished "git checkout master" &&
git checkout --orphan orphan &&
-- 
2.6.3.402.geb6a0f7

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Hello.

2015-11-21 Thread Попова Муза
База поставщиков и клиентов Вашего конкурента

Максим Архимедов

email base-concurrent3@yandex. ru

+7-929-968-9Ч-Ч3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v1.5 2/3] bash prompt: remove a redundant 'git diff' option

2015-11-21 Thread SZEDER Gábor
To get the dirty state indicator __git_ps1() runs 'git diff' with
'--quiet --exit-code' options.  '--quiet' already implies
'--exit-code', so the latter is unnecessary and can be removed.

Signed-off-by: SZEDER Gábor 
---
Reworded the Subject: line, because it sounded as if the patch were to
remove a command line option of the bash prompt script.

The rest is unchanged.

 contrib/completion/git-prompt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 07b52bedf1..7a95fbdcfd 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -476,7 +476,7 @@ __git_ps1 ()
if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&
   [ "$(git config --bool bash.showDirtyState)" != "false" ]
then
-   git diff --no-ext-diff --quiet --exit-code || w="*"
+   git diff --no-ext-diff --quiet || w="*"
if [ -n "$short_sha" ]; then
git diff-index --cached --quiet HEAD -- || i="+"
else
-- 
2.6.3.402.geb6a0f7

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Remote reference deleted on git fetch

2015-11-21 Thread Peter van der Does
Hi,

I have enabled fetch.prune and when I do a push of an existing branch
and fetch that branch the remote reference is deleted. Is this a bug or
expected behavior?

Doing the following commands on an existing repository:
$ git config fetch.prune true
$ git checkout -b bug/bug-1
Switched to a new branch 'bug/bug-1'
$ touch bugfix
$ git add .
$ git commit -a
$ git push --set-upstream origin bug/bug-1
Counting objects: 2, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (2/2), 242 bytes | 0 bytes/s, done.
Total 2 (delta 1), reused 0 (delta 0)
To g...@github.com:petervanderdoes/Testing.git
 * [new branch]  bug/bug-1 -> bug/bug-1
Branch bug/bug-1 set up to track remote branch bug/bug-1 from origin.
$ git fetch origin bug/bug-1:refs/remotes/origin/bug/bug-1
>From github.com:petervanderdoes/Testing
 x [deleted] (none) -> origin/bug/bug-1
$ git branch -r
  origin/master
$ git branch
* bug/bug-1
  master

The branch still exists on the remote repository.
Is it correct that the git fetch deletes the remote reference?


Peter

-- 
Peter van der Does

GPG key: CB317D6E

Site: http://avirtualhome.com
GitHub: https://github.com/petervanderdoes
Twitter: @petervanderdoes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC] OS X El Capitan + Xcode ships without SSL header?!

2015-11-21 Thread Lars Schneider
Hi,

I cannot build Git on a clean machine with OS X El Capitan 10.11, Xcode 7.1.1 
and Xcode command line tools because of missing OpenSSL headers.

It looks like as there are no OpenSSL headers at all. I only found this weird 
non working version:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-migrator/sdk/MacOSX.sdk/usr/include/openssl/ssl.h

I installed OpenSSL with brew, added the include path and it works.

Can anyone confirm?

Thanks,
Lars

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Downloading Git on Mac

2015-11-21 Thread Elisha Packer
For a project I am helping with, I am going through a coding program and it 
required the students to download Git. Majority of the students including 
myself are using Macs and it won’t open the program for me. I did recently 
install the new update on my computer, could this have anything to do with it?--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v1] blame: add support for --[no-]progress option

2015-11-21 Thread Edmundo Carmona Antoranz
Will also affect annotate

Signed-off-by: Edmundo Carmona Antoranz 
---
 Documentation/blame-options.txt |  7 +++
 Documentation/git-blame.txt |  9 -
 builtin/blame.c | 39 ---
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..43f4f08 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
 
+--[no-]progress::
+   Progress status is reported on the standard error stream
+   by default when it is attached to a terminal. This flag
+   enables progress reporting even if not attached to a
+   terminal.
+
+
 -M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..2e63397 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--abbrev=] [ | --contents  | --reverse ] [--] 

+   [--[no-]progress] [--abbrev=] [ | --contents  | 
--reverse ]
+   [--] 
 
 DESCRIPTION
 ---
@@ -88,6 +89,12 @@ include::blame-options.txt[]
abbreviated object name, use +1 digits. Note that 1 column
is used for a caret to mark the boundary commit.
 
+--[no-]progress::
+   Progress status is reported on the standard error stream
+   by default when it is attached to a terminal. This flag
+   enables progress reporting even if not attached to a
+   terminal.
+
 
 THE PORCELAIN FORMAT
 
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..31477d8 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
 #include "line-range.h"
 #include "line-log.h"
 #include "dir.h"
+#include "progress.h"
 
 static char blame_usage[] = N_("git blame [] [] [] 
[--] ");
 
@@ -50,6 +51,7 @@ static int incremental;
 static int xdl_opts;
 static int abbrev = -1;
 static int no_whole_file_rename;
+static int show_progress;
 
 static struct date_mode blame_date_mode = { DATE_ISO8601 };
 static size_t blame_date_width;
@@ -1760,14 +1762,28 @@ static void found_guilty_entry(struct blame_entry *ent)
}
 }
 
+static int count_blamed_lines(struct scoreboard *sb)
+{
+   int counter = 0;
+   for (struct blame_entry * entry = sb->ent; entry; entry = entry->next)
+   counter += entry->num_lines;
+   return counter;
+}
+
 /*
  * The main loop -- while we have blobs with lines whose true origin
  * is still unknown, pick one blob, and allow its lines to pass blames
  * to its parents. */
-static void assign_blame(struct scoreboard *sb, int opt)
+static void assign_blame(struct scoreboard *sb, int opt, int show_progress)
 {
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(&sb->commits);
+   struct progress * progress = NULL;
+   int blamed_lines = -1;
+
+   if (show_progress) {
+   progress = start_progress(_("Blaming lines"), sb->num_lines);
+   }
 
while (commit) {
struct blame_entry *ent;
@@ -1822,9 +1838,21 @@ static void assign_blame(struct scoreboard *sb, int opt)
}
origin_decref(suspect);
 
+   if (progress) {
+   int current_blamed_lines = count_blamed_lines(sb);
+   if (current_blamed_lines > blamed_lines) {
+   blamed_lines = current_blamed_lines;
+   display_progress(progress, blamed_lines);
+   }
+   }
+
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+   if (progress) {
+   stop_progress(&progress);
+   }
 }
 
 static const char *format_time(unsigned long time, const char *tz_str,
@@ -2520,6 +2548,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for 
boundary commits (Default: off)")),
OPT_BOOL(0, "root", &show_root, N_("Do not treat root commits 
as boundaries (Default: off)")),
OPT_BOOL(0, "show-stats", &show_stats, N_("Show work cost 
statistics")),
+   OPT_BOOL(0, "progress", &show_progress, N_("Force progress 
reporting")),
OPT_BIT(0, "score-debug", &output_option, N_("Show output score 
for blame entries"), OUTPUT_SHOW_SCORE),
OPT_BIT('f', "show-name", &output_option, N_("Show original 
file

Re: [PATCH v1] blame: add support for --[no-]progress option

2015-11-21 Thread Edmundo Carmona Antoranz
Hey, guys!

Time for some more patch destruction.

On Sat, Nov 21, 2015 at 11:11 PM, Edmundo Carmona Antoranz
 wrote:
-static void assign_blame(struct scoreboard *sb, int opt)
+static void assign_blame(struct scoreboard *sb, int opt, int show_progress)

Would it be better to include show_progress as a binary flag in opt
instead of a separate variable?

> +   struct progress * progress = NULL;
> +   int blamed_lines = -1;

I'm wondering if it would be better to save the 'last number used in
progress' inside struct progress so that we could skip blamed_lines.
That would also allow progress itself (as in the API) to avoid
printing duplicated progress values. In my case, I'm going through a
number of cycles where _I think_ I might have the same number of
blamed lines detected. To avoid asking to do a new progress print out
with the same value, I'm checking the value I had used last time. If
progress took care of that check up, this variable would go for good
and I would just ask progress to print with the current value (no
matter if it's duplicate or an increased value).

> +
> +   if (show_progress) {
> +   progress = start_progress(_("Blaming lines"), sb->num_lines);
> +   }

Already noticed I can get rid of the curly brackets.

>
> while (commit) {
> struct blame_entry *ent;
> @@ -1822,9 +1838,21 @@ static void assign_blame(struct scoreboard *sb, int 
> opt)
> }
> origin_decref(suspect);
>
> +   if (progress) {
> +   int current_blamed_lines = count_blamed_lines(sb);
> +   if (current_blamed_lines > blamed_lines) {
> +   blamed_lines = current_blamed_lines;
> +   display_progress(progress, blamed_lines);
> +   }
> +   }
> +
> if (DEBUG) /* sanity */
> sanity_check_refcnt(sb);
> }
> +
> +   if (progress) {
> +   stop_progress(&progress);
> +   }
>  }
>

It first I tried to detect how many revisions where going to be
checked so that I could report progress on them but I found extracting
information from scoreboard a little tricky (to be read: I could not
extract anything out of it). Then I though of counting lines and it
works so (same thing with the curly brackets).


> +   assign_blame(&sb, opt, show_progress);
> +
> if (!incremental)
> setup_pager();
>
> -   assign_blame(&sb, opt);
> -

setup_pager() was breaking progress so I moved it _after_
assign_blame() and it seems to behave. Hope that's not a problem.

Looking forward to your feedback.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html