Git development
 help / color / mirror / Atom feed
* Re: [OT] Re: C++ *for Git*
From: Robin Rosenberg @ 2007-09-23 23:10 UTC (permalink / raw)
  To: David Kastrup
  Cc: Dmitry Potapov, Linus Torvalds, Marco Costalba, Pierre Habouzit,
	Frank Lichtenheld, Alex Unleashed, Kyle Rose, Miles Bader,
	Dmitry Kakurin, Git
In-Reply-To: <85ejgpkr13.fsf@lola.goethe.zz>

söndag 23 september 2007 skrev David Kastrup:
> Dmitry Potapov <dpotapov@nbs-eng.ru> writes:
> 
> > On Sun, Sep 23, 2007 at 09:54:10AM -0700, Linus Torvalds wrote:
> >
> >>  - the stuff C++ *does* have is usually nasty. Implicit
> >>  initializers and destructors and the magic lifetime rules of
> >>  objects etc
> >
> > I am not sure what is wrong with initializers and destructors in
> > C++, but certainly there is no magic lifetime rules in C++, as it is
> > fully determined by the scope.
> 
> It has been some time since I last looked, but the lifetime of objects
> constructed in return statements was a moving target through several
> standards.  The last standard I bothered looking at had the object
> survive until the statement with the function call expression ended:
> quite a strange synchronization point with regard to language design.

The idea is that you should be able to use temporaries by reference and
trust them to be valid over function calls end even function returns, so 
you can write efficient matrix math libraries that do not copy data much while
retaining value semantics with overloaded operators. It is a purely practical
matter, what actually works and is efficient, not dogmatic language "design".

Earlier versions failed to make up something useful here.

> > In fact, other high level languages that use GC have much more
> > unpredictable lifetime rules for objects.
> 
> Mostly objects are alive as long as you can refer to them.  Not really
> complicated.

What could be simpler, besides all static variables.

-- robin

^ permalink raw reply

* The EGIT Chronicles Issue Volume 1, Issue 2.
From: Robin Rosenberg @ 2007-09-23 23:06 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, David Watson, Ben Konrath


Version 0.3 of the Eclipse plugin is out. 

As the zero implies this plugin doesn't do everything, but it makes your 
coffee stronger.

FEATURES

Visualization:
	See the history graphically with highlighting for commits affected
	by the selected resource (Eclipse speak for file or directory).
	Resources are decorated with status.
	Branch name on projects
	Quick diff marks you latest changes visible and revertable in the editor

Actions:
	Connect to repositories
	Add, remove files, rename, update index
	Checkout including some cases of dirty work trees
	Compare revisions using the structured compare
	Commit files selectively, amend and sign-off commits

It even works on Windows, with either cygwin or msysgit (but doesn't do CRLF 
conversions) and OS X.

For advanced stuff you still need the "Real Git" (clone, push, fetch, merge 
etc).

Special thanks to Dave Watson for the Commit and checkout stuff and Shawn 
Pearce for making it possible by doing the initial work on the plugin and 
most importantly a fast pack reader that doesn't involve JNI. The base that 
Shawn formed made it possible to extend the plugin a small piece at a time 
without knowing very much about either Eclipse or Git internals.

BUGS
	Yes, probably.

URL:
	http://repo.or.cz/w/egit.git

-- robin

^ permalink raw reply

* Re: git-send-email is omitting author and date lines
From: Junio C Hamano @ 2007-09-23 23:29 UTC (permalink / raw)
  To: Hanspeter Kunz; +Cc: git
In-Reply-To: <1190585633.29937.44.camel@localhost>

Hanspeter Kunz <hp@edelkunz.ch> writes:

> When sending a patch to myself using `git-send-email` I realized that
> the lines containing the author and the date (lines 5 and 6 in the patch
> file) were not in the sent email.

Was the commit authored by yourself?

^ permalink raw reply

* Re: git-send-email is omitting author and date lines
From: Hanspeter Kunz @ 2007-09-23 23:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4phlc668.fsf@gitster.siamese.dyndns.org>

On Sun, 2007-09-23 at 16:29 -0700, Junio C Hamano wrote:
> Hanspeter Kunz <hp@edelkunz.ch> writes:
> 
> > When sending a patch to myself using `git-send-email` I realized that
> > the lines containing the author and the date (lines 5 and 6 in the patch
> > file) were not in the sent email.
> 
> Was the commit authored by yourself?

yes. does this make a difference?

^ permalink raw reply

* [StGit PATCH] New test: make sure tha we can run "stg help"
From: Karl Hasselström @ 2007-09-23 23:51 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Karl Hasselström

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

David Kågedal pointed out that "stg help" was broken in my
experimental branch. But nothing can be expected to work if it's not
in the test suite ...

 t/t0100-help.sh |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
 create mode 100755 t/t0100-help.sh


diff --git a/t/t0100-help.sh b/t/t0100-help.sh
new file mode 100755
index 0000000..d1550ba
--- /dev/null
+++ b/t/t0100-help.sh
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+test_description='Run "stg help"'
+
+. ./test-lib.sh
+
+test_expect_success 'Run "stg help"' '
+    stg help
+    '
+
+test_expect_success 'Run "stg --help"' '
+    stg --help
+    '
+
+test_done

^ permalink raw reply related

* [StGit PATCH] Make sure that the output of "stg status" is sorted
From: Karl Hasselström @ 2007-09-23 23:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Karl Hasselström

This simplifies testing, but also makes it easier for the user to find
specific files.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/status.py |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)


diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index bbfb5df..b2835ab 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -91,12 +91,15 @@ def status(files = None, modified = False, new = False, deleted = False,
             filestat.append('?')
         cache_files = [x for x in cache_files if x[0] in filestat]
 
-    for fs in cache_files:
-        assert files == None or fs[1] in files
-        if not filtered:
-            out.stdout('%s %s' % (fs[0], fs[1]))
+    output = []
+    for st, fn in cache_files:
+        assert files == None or fn in files
+        if filtered:
+            output.append(fn)
         else:
-            out.stdout('%s' % fs[1])
+            output.append('%s %s' % (st, fn))
+    for o in sorted(output):
+        out.stdout(o)
 
 def func(parser, options, args):
     """Show the tree status

^ permalink raw reply related

* [StGit PATCH] New policy: Only use test_expect_failure for broken tests
From: Karl Hasselström @ 2007-09-23 23:55 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Karl Hasselström

New policy for tests:

  * For subtests whose commands fail when the test succeeds, use
    test_expect_success and shell negation (!). test_expect_failure is
    to be used only for subtests that are known to be broken.

  * Patches that introduce a subtest failure must change that subtest
    to use test_expect_failure.

The reason for this change is to ensure that the test suite passes at
all times, even in the middle of disruptive rewrites spread over
several commits.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

While rebasing David's conflict series on top of his new top/bottom
removal series, I found it very irritating and time-consuming to deal
with patches that intentionally broke the test suite. That won't
happen again with this policy in place.

 t/README                      |    9 ++++-----
 t/t0001-subdir-branches.sh    |    9 +++++----
 t/t1000-branch-create.sh      |    8 ++++----
 t/t1001-branch-rename.sh      |    4 ++--
 t/t1002-branch-clone.sh       |    4 ++--
 t/t1200-push-modified.sh      |    4 ++--
 t/t1202-push-undo.sh          |    4 ++--
 t/t2000-sync.sh               |    8 ++++----
 t/t2100-pull-policy-fetch.sh  |    4 ++--
 t/t2102-pull-policy-rebase.sh |   12 ++++++------
 t/t2200-rebase.sh             |    4 ++--
 11 files changed, 35 insertions(+), 35 deletions(-)


diff --git a/t/README b/t/README
index d88bad2..77f0b6c 100644
--- a/t/README
+++ b/t/README
@@ -162,11 +162,10 @@ library for your script to use.
    This is the opposite of test_expect_success.  If <script>
    yields success, test is considered a failure.
 
-   Example:
-
-	test_expect_failure \
-	    'git-update-index without --add should fail adding.' \
-	    'git-update-index should-be-empty'
+   This should _not_ be used for tests that succeed when their
+   commands fail -- use test_expect_success and shell negation (!) for
+   that. test_expect_failure is for cases when a test is known to be
+   broken.
 
  - test_debug <script>
 
diff --git a/t/t0001-subdir-branches.sh b/t/t0001-subdir-branches.sh
index 1685233..0eed3a4 100755
--- a/t/t0001-subdir-branches.sh
+++ b/t/t0001-subdir-branches.sh
@@ -38,10 +38,11 @@ test_expect_success 'Try new form of id with slashy branch' \
    stg id foo@x/y/z &&
    stg id foo@x/y/z//top'
 
-test_expect_failure 'Try old id with slashy branch' \
-  'stg id foo/ ||
-   stg id foo/top ||
-   stg id foo@x/y/z/top'
+test_expect_success 'Try old id with slashy branch' '
+   ! stg id foo/ &&
+   ! stg id foo/top &&
+   ! stg id foo@x/y/z/top
+   '
 
 test_expect_success 'Create patch in slashy branch' \
   'echo "bar" >> foo.txt &&
diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
index e920e93..848686c 100755
--- a/t/t1000-branch-create.sh
+++ b/t/t1000-branch-create.sh
@@ -18,9 +18,9 @@ test_expect_success \
     mkdir -p .git/patches && touch .git/patches/foo
 '
 
-test_expect_failure \
+test_expect_success \
     'Try to create an stgit branch with a spurious patches/ entry' '
-    stg branch -c foo
+    ! stg branch -c foo
 '
 
 test_expect_success \
@@ -35,9 +35,9 @@ test_expect_success \
     cp .git/refs/heads/master .git/refs/heads/foo
 '
 
-test_expect_failure \
+test_expect_success \
     'Try to create an stgit branch with an existing git branch by that name' '
-    stg branch -c foo
+    ! stg branch -c foo
 '
 
 test_expect_success \
diff --git a/t/t1001-branch-rename.sh b/t/t1001-branch-rename.sh
index 285440f..dd12132 100755
--- a/t/t1001-branch-rename.sh
+++ b/t/t1001-branch-rename.sh
@@ -17,9 +17,9 @@ test_expect_success \
      stg new p1 -m "p1"
 '
 
-test_expect_failure \
+test_expect_success \
     'Rename the current stgit branch' \
-    'stg branch -r foo bar
+    '! stg branch -r foo bar
 '
 
 test_expect_success \
diff --git a/t/t1002-branch-clone.sh b/t/t1002-branch-clone.sh
index 1d7fc39..b0087e9 100755
--- a/t/t1002-branch-clone.sh
+++ b/t/t1002-branch-clone.sh
@@ -18,10 +18,10 @@ test_expect_success \
     git commit -a -m bar
     '
 
-test_expect_failure \
+test_expect_success \
     'Try to create a patch in a GIT branch' \
     '
-    stg new p0 -m "p0"
+    ! stg new p0 -m "p0"
     '
 
 test_expect_success \
diff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh
index 0e408d0..cfec696 100755
--- a/t/t1200-push-modified.sh
+++ b/t/t1200-push-modified.sh
@@ -47,9 +47,9 @@ test_expect_success \
      )
 "
 
-test_expect_failure \
+test_expect_success \
     'Attempt to push the first of those patches without --merged' \
-    "(cd bar && stg push
+    "(cd bar && ! stg push
      )
 "
 
diff --git a/t/t1202-push-undo.sh b/t/t1202-push-undo.sh
index 335b554..039103a 100755
--- a/t/t1202-push-undo.sh
+++ b/t/t1202-push-undo.sh
@@ -40,10 +40,10 @@ test_expect_success \
 	stg pop --all
 	'
 
-test_expect_failure \
+test_expect_success \
 	'Push the second patch with conflict' \
 	'
-	stg push bar
+	! stg push bar
 	'
 
 test_expect_success \
diff --git a/t/t2000-sync.sh b/t/t2000-sync.sh
index f831df7..484dbab 100755
--- a/t/t2000-sync.sh
+++ b/t/t2000-sync.sh
@@ -106,10 +106,10 @@ test_expect_success \
     test $(cat bar2.txt) = "bar2"
     '
 
-test_expect_failure \
+test_expect_success \
     'Synchronise the first two patches with the master branch (to fail)' \
     '
-    stg sync -B master -a
+    ! stg sync -B master -a
     '
 
 test_expect_success \
@@ -124,10 +124,10 @@ test_expect_success \
     [ "$(echo $(stg unapplied))" = "" ]
     '
 
-test_expect_failure \
+test_expect_success \
     'Synchronise the third patch with the exported series (to fail)' \
     '
-    stg sync -s patches-master/series p3
+    ! stg sync -s patches-master/series p3
     '
 
 test_expect_success \
diff --git a/t/t2100-pull-policy-fetch.sh b/t/t2100-pull-policy-fetch.sh
index e1398a3..1f50069 100755
--- a/t/t2100-pull-policy-fetch.sh
+++ b/t/t2100-pull-policy-fetch.sh
@@ -65,8 +65,8 @@ test_expect_success \
     (cd clone && stg commit && stg new c2 -m c2 &&
      echo a >> file && stg refresh)
     '
-test_expect_failure \
+test_expect_success \
     'Try to  and commit a patch in clone' \
-    '(cd clone && stg pull)'
+    '(cd clone && ! stg pull)'
 
 test_done
diff --git a/t/t2102-pull-policy-rebase.sh b/t/t2102-pull-policy-rebase.sh
index 670673d..b2fbfcf 100755
--- a/t/t2102-pull-policy-rebase.sh
+++ b/t/t2102-pull-policy-rebase.sh
@@ -46,18 +46,18 @@ test_expect_success \
     stg branch stack && stg commit && stg new c2 -m c2 &&
      echo a >> file && stg refresh
     '
-test_expect_failure \
+test_expect_success \
     'Try to pull/rebase now that stack base has moved' \
-    'stg pull'
+    '! stg pull'
 
 test_expect_success \
     'Force the pull/rebase, but do not push yet' \
     'stg pull --force --nopush'
-test_expect_failure \
+test_expect_success \
     '...check we lost the committed patch' \
-    'test -e file'
-test_expect_failure \
+    '! test -e file'
+test_expect_success \
     '...and check we get a conflict while pushing' \
-    'stg push'
+    '! stg push'
 
 test_done
diff --git a/t/t2200-rebase.sh b/t/t2200-rebase.sh
index c142e08..ec2a104 100755
--- a/t/t2200-rebase.sh
+++ b/t/t2200-rebase.sh
@@ -31,10 +31,10 @@ test_expect_success \
 	test `stg applied | wc -l` = 1
 	'
 
-test_expect_failure \
+test_expect_success \
 	'Attempt rebase to non-existing commit' \
 	'
-	stg rebase not-a-ref
+	! stg rebase not-a-ref
 	'
 
 test_expect_success \

^ permalink raw reply related

* Re: git-rebase--interactive needs a better message
From: Johannes Schindelin @ 2007-09-23 23:56 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: git
In-Reply-To: <20070923224502.GB7249@potapov>

Ho.

On Mon, 24 Sep 2007, Dmitry Potapov wrote:

> I have tried to use git-rebase --interactive today, and run into a strange
> error message saying:
> 
> /usr/bin/git-rebase--interactive: line 333: $GIT_DIR/.dotest-merge/author-script: No such file or directory
> 
> I had to scratch my head for a while before I realized that I forgot to
> say git-commit. So, it was mine mistake, but I think that it should be
> possible to have a better error message suggesting the user what to do.

Actually, it should just work, I'd say.  IOW git-rebase--interactive 
should store an author script also for "edit"s.

Ciao,
Dscho

^ permalink raw reply

* StGit kha safe branch updated
From: Karl Hasselström @ 2007-09-24  0:02 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

These are the first ten patches of David's latest series (including
the fixup he suggested) -- that is, only the cleanup part -- plus the
three patches I just mailed out.

The following changes since commit ead8e89297dfabf7c290338a86c89f19c5ba7401:
  Catalin Marinas (1):
        Print 'updating patch' earlier in 'edit'

are available in the git repository at:

  git://repo.or.cz/stgit/kha.git safe

David Kågedal (10):
      Add some more tests of "stg status" output
      Clear up semantics of tree_status
      Moved that status function to the status command file
      Split Series.push_patch in two
      Remove dead code from push_empty_patch
      Refactor Series.push_patch
      Clean up Series.refresh_patch
      Add a 'bottom' parameter to Series.refresh_patch and use it
      Clear up the semantics of Series.new_patch
      Refactor Series.new_patch

Karl Hasselström (3):
      New test: make sure tha we can run "stg help"
      Make sure that the output of "stg status" is sorted
      New policy: Only use test_expect_failure for broken tests

 stgit/commands/common.py      |    2 +-
 stgit/commands/status.py      |   45 +++++++++++-
 stgit/commands/uncommit.py    |    1 +
 stgit/git.py                  |   61 ++++------------
 stgit/stack.py                |  155 ++++++++++++++++++++++------------------
 t/README                      |    9 +--
 t/t0001-subdir-branches.sh    |    9 ++-
 t/t0002-status.sh             |   36 ++++++++++
 t/t0100-help.sh               |   15 ++++
 t/t1000-branch-create.sh      |    8 +-
 t/t1001-branch-rename.sh      |    4 +-
 t/t1002-branch-clone.sh       |    4 +-
 t/t1200-push-modified.sh      |    4 +-
 t/t1202-push-undo.sh          |    4 +-
 t/t2000-sync.sh               |    8 +-
 t/t2100-pull-policy-fetch.sh  |    4 +-
 t/t2102-pull-policy-rebase.sh |   12 ++--
 t/t2200-rebase.sh             |    4 +-
 18 files changed, 230 insertions(+), 155 deletions(-)
 create mode 100755 t/t0100-help.sh

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* StGit kha experimental branch updated
From: Karl Hasselström @ 2007-09-24  0:10 UTC (permalink / raw)
  To: git
In-Reply-To: <20070924000244.GA24403@diana.vm.bytemark.co.uk>

The three last patches of David's top/bottom removal series are here,
and all the old patches too. Changes:

  * Everything's been rebased on top of David's top/bottom removal
    series, which turned out to be a major undertaking. I'm not 100%
    sure I've managed to not break anything.

  * I've modified the conflict series to adhere to the new test suite
    policy; it now passes the test suite at every point, by using
    test_expect_failure for broken tests.

  * The add/rm/cp removal patches no longer break "stg help".

The following changes since commit 5f594e90e9a0867e1b87ffb8b0a077152246b56c:
  Karl Hasselström (1):
        New policy: Only use test_expect_failure for broken tests

are available in the git repository at:

  git://repo.or.cz/stgit/kha.git experimental

David Kågedal (9):
      Check bottom and invariants
      Remove the 'bottom' field
      Remove the 'top' field
      Split git.merge into two functions
      Leave working dir and index alone after failed (conflicting) push
      Added a test case to check what happens when push finds a conflict
      Simplify merge_recursive
      Use the output from merge-recursive to list conflicts
      Ask git about unmerged files

Karl Hasselström (20):
      Remove the --force flag to "stg rebase" and "stg pull"
      Better error message if merge fails
      Fix "stg resolved" to work with new conflict representation
      Refactoring: pass more than one file to resolved()
      We keep the different stages of a conflict in the index now
      Clean up the logic in "stg resolved"
      "stg status --reset" is not needed anymore
      Remove "stg add"
      Remove "stg rm"
      Remove "stg cp"
      Compute patch appliedness from commit DAG
      Test the new DAG appliedness machinery
      Fix bash completion after the DAG appliedness patch
      Speed up the appliedness test
      Speed up the discovery of uninteresting commits
      Speed up appliedness check during patch creation
      Don't traverse the whole DAG when looking for uninteresting commits
      Find uninteresting commits faster for special cases
      Optimize uninterestingness checks for rebase
      Merge branch 'conflict' into experimental

 Documentation/stg-cp.txt      |   63 -----
 Documentation/tutorial.txt    |   22 +-
 contrib/stgit-completion.bash |   15 +-
 stgit/commands/add.py         |   43 ---
 stgit/commands/commit.py      |    8 +-
 stgit/commands/common.py      |   34 +--
 stgit/commands/copy.py        |   44 ---
 stgit/commands/float.py       |    2 +-
 stgit/commands/pick.py        |    2 +-
 stgit/commands/pull.py        |    5 +-
 stgit/commands/rebase.py      |    5 +-
 stgit/commands/resolved.py    |   70 ++---
 stgit/commands/rm.py          |   47 ---
 stgit/commands/status.py      |   34 +--
 stgit/commands/sync.py        |    1 -
 stgit/git.py                  |   72 +++--
 stgit/gitmergeonefile.py      |   99 ++++---
 stgit/main.py                 |    6 -
 stgit/run.py                  |    3 +
 stgit/stack.py                |  620 ++++++++++++++++++++++++++++++++---------
 t/t0002-status.sh             |   11 +-
 t/t1200-push-modified.sh      |    2 +-
 t/t1202-push-undo.sh          |    4 +-
 t/t1203-push-conflict.sh      |   70 +++++
 t/t1300-uncommit.sh           |    4 +-
 t/t1301-assimilate.sh         |    2 +-
 t/t1400-patch-history.sh      |    4 +-
 t/t1500-float.sh              |   14 +-
 t/t1600-delete-one.sh         |   12 +-
 t/t1601-delete-many.sh        |    2 +-
 t/t1700-goto-top.sh           |    2 +-
 t/t2000-sync.sh               |    8 +-
 t/t2100-pull-policy-fetch.sh  |   18 +-
 t/t2101-pull-policy-pull.sh   |    4 +-
 t/t2102-pull-policy-rebase.sh |   28 +--
 t/t3000-git-interop.sh        |   60 ++++
 t/t4000-upgrade.sh            |    6 +
 37 files changed, 822 insertions(+), 624 deletions(-)
 delete mode 100644 Documentation/stg-cp.txt
 delete mode 100644 stgit/commands/add.py
 delete mode 100644 stgit/commands/copy.py
 delete mode 100644 stgit/commands/rm.py
 create mode 100755 t/t1203-push-conflict.sh
 create mode 100755 t/t3000-git-interop.sh

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* [PATCH] rebase -i: commit when continuing after "edit"
From: Johannes Schindelin @ 2007-09-24  0:29 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: git
In-Reply-To: <20070923224502.GB7249@potapov>


When doing an "edit" on a commit, editing and git-adding some files,
"git rebase -i" complained about a missing "author-script".  The idea was 
that the user would call "git commit --amend" herself.

But we can be nice and do that for the user.

To do this, rebase -i stores the author script and message whenever 
writing out a patch, and it remembers to do an "amend" by creating the 
file "amend" in "$DOTEST".

Noticed by Dmitry Potapov.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Mon, 24 Sep 2007, Dmitry Potapov wrote:

	> I have tried to use git-rebase --interactive today, and run into 
	> a strange error message saying:
	> 
	> /usr/bin/git-rebase--interactive: \
	>	line 333: $GIT_DIR/.dotest-merge/author-script: \
	>		No such file or directory

	Could you please apply this patch and try if the issue is gone?

	The patch looks a bit strange, because some code moved from 
	die_with_patch() to make_patch(), and the diff makes it look like 
	the end of the function moved instead.

	Funnily enough we discussed human readable diffs briefly on #git
	today, but I think even the best diff algorithms could not catch 
	that.

 git-rebase--interactive.sh    |   12 ++++++++----
 t/t3404-rebase-interactive.sh |   14 +++++++++++++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8258b7a..e4cf282 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -79,13 +79,13 @@ mark_action_done () {
 make_patch () {
 	parent_sha1=$(git rev-parse --verify "$1"^ 2> /dev/null)
 	git diff "$parent_sha1".."$1" > "$DOTEST"/patch
-}
-
-die_with_patch () {
 	test -f "$DOTEST"/message ||
 		git cat-file commit $sha1 | sed "1,/^$/d" > "$DOTEST"/message
 	test -f "$DOTEST"/author-script ||
 		get_author_ident_from_commit $sha1 > "$DOTEST"/author-script
+}
+
+die_with_patch () {
 	make_patch "$1"
 	die "$2"
 }
@@ -214,6 +214,7 @@ peek_next_command () {
 do_next () {
 	test -f "$DOTEST"/message && rm "$DOTEST"/message
 	test -f "$DOTEST"/author-script && rm "$DOTEST"/author-script
+	test -f "$DOTEST"/amend && rm "$DOTEST"/amend
 	read command sha1 rest < "$TODO"
 	case "$command" in
 	\#|'')
@@ -233,6 +234,7 @@ do_next () {
 		pick_one $sha1 ||
 			die_with_patch $sha1 "Could not apply $sha1... $rest"
 		make_patch $sha1
+		: > "$DOTEST"/amend
 		warn
 		warn "You can amend the commit now, with"
 		warn
@@ -332,7 +334,9 @@ do
 		git update-index --refresh &&
 		git diff-files --quiet &&
 		! git diff-index --cached --quiet HEAD &&
-		. "$DOTEST"/author-script &&
+		. "$DOTEST"/author-script && {
+			test ! -f "$DOTEST"/amend || git reset --soft HEAD^
+		} &&
 		export GIT_AUTHOR_NAME GIT_AUTHOR_NAME GIT_AUTHOR_DATE &&
 		git commit -F "$DOTEST"/message -e
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 718c9c1..1af73a4 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -80,7 +80,7 @@ cat "$1".tmp
 action=pick
 for line in $FAKE_LINES; do
 	case $line in
-	squash)
+	squash|edit)
 		action="$line";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
@@ -297,4 +297,16 @@ test_expect_success 'ignore patch if in upstream' '
 	test $HEAD = $(git rev-parse HEAD^)
 '
 
+test_expect_success '--continue tries to commit, even for "edit"' '
+	parent=$(git rev-parse HEAD^) &&
+	test_tick &&
+	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
+	echo edited > file7 &&
+	git add file7 &&
+	FAKE_COMMIT_MESSAGE="chouette!" git rebase --continue &&
+	test edited = $(git show HEAD:file7) &&
+	git show HEAD | grep chouette &&
+	test $parent = $(git rev-parse HEAD^)
+'
+
 test_done
-- 
1.5.3.2.1039.g855b8

^ permalink raw reply related

* Re: git-send-email is omitting author and date lines
From: Johannes Schindelin @ 2007-09-24  0:30 UTC (permalink / raw)
  To: Hanspeter Kunz; +Cc: Junio C Hamano, git
In-Reply-To: <1190590538.12557.1.camel@localhost>

Hi,

On Mon, 24 Sep 2007, Hanspeter Kunz wrote:

> On Sun, 2007-09-23 at 16:29 -0700, Junio C Hamano wrote:
> > Hanspeter Kunz <hp@edelkunz.ch> writes:
> > 
> > > When sending a patch to myself using `git-send-email` I realized that
> > > the lines containing the author and the date (lines 5 and 6 in the patch
> > > file) were not in the sent email.
> > 
> > Was the commit authored by yourself?
> 
> yes. does this make a difference?

Yes, it does.  The author is usually inferred from the sender of the mail, 
and therefore git-send-email strips that information from the message 
(IIRC).

Hth,
Dscho

^ permalink raw reply

* Re: [PATCH] git-submodule - allow a relative path as the subproject url
From: Michael Smith @ 2007-09-24  0:21 UTC (permalink / raw)
  To: git
In-Reply-To: <11904936042891-git-send-email-mdl123@verizon.net>

Mark Levedahl <mdl123 <at> verizon.net> writes:

> This allows a subproject's location to be specified and stored as relative
> to the parent project's location (e.g., ./foo, or ../foo). This url is
> stored in .gitmodules as given. 

> This allows cloning of the project to work "as expected" if the project
> is hosted on a different server than when the subprojects were added.

Hi Mark,

I have a use case where this would be very handy -- two teams geographically
separated. But in the end I think I disagree with it: it's not up to me, as the
person creating a superproject, to decide that anyone who forks or clones it
also has to clone all the submodules.

Instead they can decide themselves by editing .git/config between "git submodule
init" and "git submodule update", or by branching .gitmodules.

By the way:

> +# Get parent project's url
> +get_parent_url ()
> +{
> +	# need to append this on parent project's url
> +	branch="$(git branch --no-color | sed -ne 's/^\* //p')"
> +	test -n "$branch" || die "I do not know what branch you are on: $branch"
> +	upstream="$(git config branch.$branch.remote)"
> +	test -n "$upstream" || die "Cannot find upstream repo for branch $branch"
> +	uprepo="$(git config remote.$upstream.url)"
> +	test -n "$upstream" || die "Cannot find url for repo $uprepo"
> +	echo "$uprepo"
> +}
...
> +	case $repo in
> +	.*)
> +		realrepo="$(get_parent_url)/$repo" ;;

If you source git-parse-remote, you can use

                realrepo="$(get_remote_url "$(get_default_remote)")$repo"

Mike

^ permalink raw reply

* [PATCH] git-submodule - allow a relative path as the subproject url
From: Mark Levedahl @ 2007-09-24  2:19 UTC (permalink / raw)
  To: git; +Cc: Mark Levedahl
In-Reply-To: <11904936042891-git-send-email-mdl123@verizon.net>

This allows a subproject's location to be specified and stored as relative
to the parent project's location (e.g., ./foo, or ../foo). This url is
stored in .gitmodules as given. It is resolved into an absolute url by
appending it to the parent project's url when the information is written
to .git/config (i.e., during submodule add for the originator, and
submodule init for a downstream recipient). This allows cloning of the
project to work "as expected" if the project is hosted on a different
server than when the subprojects were added.

Signed-off-by: Mark Levedahl <mdl123@verizon.net>
---
This patch is slightly improved as it removes embedded ./ and ../ sequences
from the url constructed from a base and relative part. It also avoids use
of git-branch in favor of git-symbolic-ref.

 Documentation/git-submodule.txt |    3 ++
 git-submodule.sh                |   53 ++++++++++++++++++++++++++++++++++----
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 2c48936..d421677 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -21,6 +21,9 @@ add::
 	repository is cloned at the specified path, added to the
 	changeset and registered in .gitmodules.   If no path is
 	specified, the path is deduced from the repository specification.
+        If the repository url begins with ./ or ../, it is stored as
+        given but resolved as a relative path from the main project's
+        url when cloning.

 status::
 	Show the status of the submodules. This will print the SHA-1 of the
diff --git a/git-submodule.sh b/git-submodule.sh
index 3320998..5bf4551 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -39,6 +39,32 @@ get_repo_base() {
 	) 2>/dev/null
 }

+# Resolve relative url by appending to parent's url
+resolve_relative_url ()
+{
+	branch="$(git symbolic-ref HEAD 2>/dev/null)"
+	remote="$(git config branch.${branch#refs/heads/}.remote)"
+	remote="${remote:-origin}"
+	remoteurl="$(git config remote.$remote.url)" ||
+		die "remote ($remote) does not have a url in .git/config"
+	url="$1"
+	while test -n "$url"
+	do
+		case "$url" in
+		../*)
+			url="${url#../}"
+			remoteurl="${remoteurl%/*}"
+			;;
+		./*)
+			url="${url#./}"
+			;;
+		*)
+			break;;
+		esac
+	done
+	echo "$remoteurl/$url"
+}
+
 #
 # Map submodule path to submodule name
 #
@@ -103,11 +129,19 @@ module_add()
 		usage
 	fi

-	# Turn the source into an absolute path if
-	# it is local
-	if base=$(get_repo_base "$repo"); then
-		repo="$base"
-	fi
+	case $repo in
+	./*|../*)
+		# dereference source url relative to parent's url
+		realrepo="$(resolve_relative_url $repo)" ;;
+	*)
+		# Turn the source into an absolute path if
+		# it is local
+		if base=$(get_repo_base "$repo"); then
+			repo="$base"
+			realrepo=$repo
+		fi
+		;;
+	esac

 	# Guess path from repo if not specified or strip trailing slashes
 	if test -z "$path"; then
@@ -122,7 +156,7 @@ module_add()
 	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
 	die "'$path' already exists in the index"

-	module_clone "$path" "$repo" || exit
+	module_clone "$path" "$realrepo" || exit
 	(unset GIT_DIR && cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
 	die "Unable to checkout submodule '$path'"
 	git add "$path" ||
@@ -153,6 +187,13 @@ modules_init()
 		test -z "$url" &&
 		die "No url found for submodule path '$path' in .gitmodules"

+		# Possibly a url relative to parent
+		case $url in
+		./*|../*)
+			url="$(resolve_relative_url $url)"
+			;;
+		esac
+
 		git config submodule."$name".url "$url" ||
 		die "Failed to register url for submodule path '$path'"

--
1.5.3.2.9.g69b29

^ permalink raw reply related

* Re: [PATCH] git-submodule - allow a relative path as the subproject url
From: Mark Levedahl @ 2007-09-24  2:27 UTC (permalink / raw)
  To: Michael Smith; +Cc: git
In-Reply-To: <loom.20070924T002011-331@post.gmane.org>

Michael Smith wrote:
> Hi Mark,
>
> I have a use case where this would be very handy -- two teams geographically
> separated. But in the end I think I disagree with it: it's not up to me, as the
> person creating a superproject, to decide that anyone who forks or clones it
> also has to clone all the submodules.
>
>   
With or without the patch, the cloner has to execute "submodule init" 
and "submodule update" to actually check things out. Absolutely nothing 
has changed there: it still requires the same explicit actions to get 
the submodules.

This patch does not alter any existing behavior: it provides an 
additional option, and one that is very useful in a world of mirrors. My 
particular use case is for a geographically distributed work flow 
involving several mirrors, all of which are behind firewalls and/or 
connected only to a LAN. There is no single server that is available to 
everyone, hence no single url for a submodule is usable across the 
group. By offering the *option* (and it is only an option) to make the 
urls relative to the super-project, the super-project can be usefully 
shared.
> Instead they can decide themselves by editing .git/config between "git submodule
> init" and "git submodule update", or by branching .gitmodules.
>   
Yes, a person can edit .gitmodules, or can edit .git/config, in order to 
tell git where things are. But why should they *have* to when relative 
urls will suffice and git can take care of this transparently.
> If you source git-parse-remote, you can use
>
>                 realrepo="$(get_remote_url "$(get_default_remote)")$repo"
>
> Mike
>   
Yes, the patch itself can be improved, am sending a revised one.

Mark

^ permalink raw reply

* Re: user-manual changes
From: Michael Smith @ 2007-09-24  2:32 UTC (permalink / raw)
  To: git; +Cc: J. Bruce Fields, Miklos Vajna


J. Bruce Fields wrote:

> The submodule documentation looks fine to me.  My only suggestion is
> that it should start with a brief explanation of why we have submodules
> and what somebody would use them for.

OK, I'll bite.

^ permalink raw reply

* Re: The msysGit Herald, issue 2
From: Shawn O. Pearce @ 2007-09-24  2:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: msysgit, git
In-Reply-To: <Pine.LNX.4.64.0709232153230.28395@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> Maybe we'll get to the state where a link to git gui is installed in
> addition to the "Git Shell"

I'm hoping that becomes possible as part of the 0.9.x series of
git-gui.  My latest master branch in git-gui.git contains a new
startup wizard that can be used to do one of three common actions:

  - Clone a repository
  - Create a new repository
  - Open an existing repository

Dscho has already seen this wizard.  Its certainly meant to help
with linking to git-gui from "Begin" menus on systems that probably
should have been called "Wide Open Doors" (given their security
track record).  Currently the only way to get the wizard to launch
is to run `git gui` from a pwd that isn't contained within a Git
repository.
 
> > 5) What was the most surprising moment when working with Git?
> 
> It merged a change I made in an old branch in a file that was moved in
> the new branch to a new location. IOW, merge across renames. Really, I
> was so surprised - I wanted to start the manual merge when I discovered
> that git had done all the work for me.

Heh.  That always gets me too.  Especially when applying git-gui
patches made against git.git, but to my git-gui tree.  `git-am -3`
Just Does The Right Thing(tm).  I can't ask for more from a source
code control system.
 
> > 6) What was the most frustrating moment when working with Git?
> 
> Just the other day, I wanted to fetch a set of changes from a public
> repo into my test repo in order to cherry-pick from them - and it
> automatically fetched all the tags. But, the heck, I don't want them tags
> here, just the commits. I just can't figure out how to avoid the automatic
> fetching of tags.

Its called `git fetch --no-tags`.  You can also setup a config
option of "remote.$name.tagopt = --no-tags" to tell git-fetch to
not fetch tags when fetching from that remote.

But yea, I too hate the fact that Git cannot read my mind and realize
that although I usually do want to fetch tags, right now I don't,
even though I forgot to tell it that with --no-tags.  :)

So my most frustrating moment when working with Git was realizing
that Linus didn't supply `git-read-users-mind`.  Sadly it only
comes with `git-read-tree`.  Ah, well, at least my maple and pine
are understood.
 
-- 
Shawn.

^ permalink raw reply

* [PATCH] user-manual: Explain what submodules are good for.
From: Michael Smith @ 2007-09-24  3:14 UTC (permalink / raw)
  To: git; +Cc: J. Bruce Fields, Miklos Vajna, Michael Smith
In-Reply-To: <20070923172702.GA5916@fieldses.org>

Rework the introduction to the Submodules section to explain why
someone would use them, and fix up submodule references from the
tree-object and todo sections.

Signed-off-by: Michael Smith <msmith@cbnco.com>
---
 Documentation/user-manual.txt |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index a085ca1..bd77e62 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -2856,8 +2856,7 @@ between two related tree objects, since it can ignore any entries with
 identical object names.
 
 (Note: in the presence of submodules, trees may also have commits as
-entries.   See gitlink:git-submodule[1] and gitlink:gitmodules.txt[1]
-for partial documentation.)
+entries.  See <<submodules>> for documentation.)
 
 Note that the files all have mode 644 or 755: git actually only pays
 attention to the executable bit.
@@ -3163,12 +3162,18 @@ information as long as you have the name of the tree that it described.
 Submodules
 ==========
 
-This tutorial explains how to create and publish a repository with submodules
-using the gitlink:git-submodule[1] command.
+Some large projects are composed of smaller, self-contained parts.  For
+example, an embedded Linux distribution's source tree would include every
+piece of software in the distribution; a movie player might need to build
+against a specific, known-working version of a decompression library;
+several independent programs might all share the same build scripts.
 
-Submodules maintain their own identity; the submodule support just stores the
-submodule repository location and commit ID, so other developers who clone the
-superproject can easily clone all the submodules at the same revision.
+Git's submodule support allows a repository to contain, as a subdirectory, a
+checkout of an external project.  Submodules maintain their own identity;
+the submodule support just stores the submodule repository location and
+commit ID, so other developers who clone the superproject can easily clone
+all the submodules at the same revision.  The gitlink:git-submodule[1]
+command manages submodules.
 
 To see how submodule support works, create (for example) four example
 repositories that can be used later as a submodule:
@@ -3213,8 +3218,8 @@ The `git submodule add` command does a couple of things:
 
 - It clones the submodule under the current directory and by default checks out
   the master branch.
-- It adds the submodule's clone path to the `.gitmodules` file and adds this
-  file to the index, ready to be committed.
+- It adds the submodule's clone path to the gitlink:gitmodules[5] file and
+  adds this file to the index, ready to be committed.
 - It adds the submodule's current commit ID to the index, ready to be
   committed.
 
@@ -4277,5 +4282,3 @@ Write a chapter on using plumbing and writing scripts.
 Alternates, clone -reference, etc.
 
 git unpack-objects -r for recovery
-
-submodules
-- 
1.5.3

^ permalink raw reply related

* Re: [EGIT PATCH] Change to simplified icon.
From: Shawn O. Pearce @ 2007-09-24  4:17 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Ben Konrath, git, David Watson
In-Reply-To: <200709192259.10054.robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> onsdag 19 september 2007 skrev Robin Rosenberg:
> > onsdag 19 september 2007 skrev Robin Rosenberg:
> > > I'll leave this one in my patch tree for now. The Egit icon contest is stil 
> > open to all.
> > 
> > Here's another icon. The cup inspired the drug used to code the plugin. The 
> > +/-'s from the git logo, the blue color to look better with the other icons 
> > in Eclipse's about box and the rest comes form the fact that I cannot draw.
> > 
> And the subject matter :)

Very cute.  I actually like it.  Maybe I'll take a stab at creating
a git-gui icon, because like you I also cannot draw.  A feather
and some pluses and minuses can't be that hard, can it?  :)

Actually I just need to find a two year old.  Any two year old with
crayons can draw many times better than I ever could.  However I
think the difficult part is to get the two year old to draw the
subject matter...

-- 
Shawn.

^ permalink raw reply

* [PATCH] git-remote rm: add tests and minor fix-ups
From: Junio C Hamano @ 2007-09-24  5:39 UTC (permalink / raw)
  To: git; +Cc: James Bowes

This fixes "git remote rm" which always exited with a failure,
corrects indentation, and adds tests.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * People seemed to have liked "git remote rm"; additional tests
   were requested to complete the series when the patch was
   submitted but they never came, so I ended up doing the
   janitorial work myself, which led to discovery of breakage in
   the implementation.

   Needless to say, I am not overly amused.

 git-remote.perl   |   10 ++---
 t/t5505-remote.sh |  100 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 6 deletions(-)
 create mode 100755 t/t5505-remote.sh

diff --git a/git-remote.perl b/git-remote.perl
index f513a8a..b7c1e01 100755
--- a/git-remote.perl
+++ b/git-remote.perl
@@ -317,7 +317,7 @@ sub update_remote {
 }
 
 sub rm_remote {
-    my ($name) = @_;
+	my ($name) = @_;
 	if (!exists $remote->{$name}) {
 		print STDERR "No such remote $name\n";
 		return;
@@ -336,7 +336,7 @@ sub rm_remote {
 	};
 
 
-    my @refs = $git->command('for-each-ref',
+	my @refs = $git->command('for-each-ref',
 		'--format=%(refname) %(objectname)', "refs/remotes/$name");
 	for (@refs) {
 		($ref, $object) = split;
@@ -453,11 +453,9 @@ elsif ($ARGV[0] eq 'add') {
 elsif ($ARGV[0] eq 'rm') {
 	if (@ARGV <= 1) {
 		print STDERR "Usage: git remote rm <remote>\n";
+		exit(1);
 	}
-    else {
-        rm_remote($ARGV[1]);
-	}
-    exit(1);
+	rm_remote($ARGV[1]);
 }
 else {
 	print STDERR "Usage: git remote\n";
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
new file mode 100755
index 0000000..636aec2
--- /dev/null
+++ b/t/t5505-remote.sh
@@ -0,0 +1,100 @@
+#!/bin/sh
+
+test_description='git remote porcelain-ish'
+
+. ./test-lib.sh
+
+GIT_CONFIG=.git/config
+export GIT_CONFIG
+
+setup_repository () {
+	mkdir "$1" && (
+	cd "$1" &&
+	git init &&
+	>file &&
+	git add file &&
+	git commit -m "Initial" &&
+	git checkout -b side &&
+	>elif &&
+	git add elif &&
+	git commit -m "Second" &&
+	git checkout master
+	)
+}
+
+tokens_match () {
+	echo "$1" | tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
+	echo "$2" | tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
+	diff -u expect actual
+}
+
+check_remote_track () {
+	actual=$(git remote show "$1" | sed -n -e '$p') &&
+	shift &&
+	tokens_match "$*" "$actual"
+}
+
+check_tracking_branch () {
+	f="" &&
+	r=$(git for-each-ref "--format=%(refname)" |
+		sed -ne "s|^refs/remotes/$1/||p") &&
+	shift &&
+	tokens_match "$*" "$r"
+}
+
+test_expect_success setup '
+
+	setup_repository one &&
+	setup_repository two &&
+	(
+		cd two && git branch another
+	) &&
+	git clone one test
+
+'
+
+test_expect_success 'remote information for the origin' '
+(
+	cd test &&
+	tokens_match origin "$(git remote)" &&
+	check_remote_track origin master side &&
+	check_tracking_branch origin HEAD master side
+)
+'
+
+test_expect_success 'add another remote' '
+(
+	cd test &&
+	git remote add -f second ../two &&
+	tokens_match "origin second" "$(git remote)" &&
+	check_remote_track origin master side &&
+	check_remote_track second master side another &&
+	check_tracking_branch second master side another &&
+	git for-each-ref "--format=%(refname)" refs/remotes |
+	sed -e "/^refs\/remotes\/origin\//d" \
+	    -e "/^refs\/remotes\/second\//d" >actual &&
+	>expect &&
+	diff -u expect actual
+)
+'
+
+test_expect_success 'remove remote' '
+(
+	cd test &&
+	git remote rm second
+)
+'
+
+test_expect_success 'remove remote' '
+(
+	cd test &&
+	tokens_match origin "$(git remote)" &&
+	check_remote_track origin master side &&
+	git for-each-ref "--format=%(refname)" refs/remotes |
+	sed -e "/^refs\/remotes\/origin\//d" >actual &&
+	>expect &&
+	diff -u expect actual
+)
+'
+
+test_done
-- 
1.5.3.2.1053.g5065

^ permalink raw reply related

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: Mike Hommey @ 2007-09-24  6:05 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <85myvdktb3.fsf@lola.goethe.zz>

On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
> -while case $# in 0) break ;; esac
> +while test $# != 0

Wouldn't -ne be better ?

Mike

^ permalink raw reply

* Re: [PATCH] [git-p4] Detect exec bit in more cases.
From: Simon Hausmann @ 2007-09-24  6:20 UTC (permalink / raw)
  To: David Brown; +Cc: Junio C Hamano, git
In-Reply-To: <20070921212443.GA14983@old.davidb.org>

[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]

On Friday 21 September 2007 23:24:43 David Brown wrote:
> On Fri, Sep 21, 2007 at 02:15:10PM -0700, Junio C Hamano wrote:
> >Simon Hausmann <simon@lst.de> writes:
> >> On Friday 21 September 2007 00:53:52 Junio C Hamano wrote:
> >>> David Brown <git@davidb.org> writes:
> >>> > On Wed, Sep 19, 2007 at 09:03:50PM +0200, Simon Hausmann wrote:
> >>> >>On Wednesday 19 September 2007 20:15:03 David Brown wrote:
> >>> >>> git-p4 was missing the execute bit setting if the file had other
> >>> >>> attribute bits set.
> >>> >>> ---
> >>> >>
> >>> >>I'm fine with this, so unless you find a better way:
> >>> >>
> >>> >>Acked-By: Simon Hausmann <simon@lst.de>
> >>> >
> >>> > I sent out an improved version of this patch yesterday
> >>> > <1190232768445-git-send-email-git@davidb.org> that I'd like to get
> >>> > approved.  I guess I'm not quite sure what happens at this point with
> >>> > a patch.
> >>>
> >>> I still have that *768445* message as "the last one proposed as
> >>> better than previous ones" in my mbox.
> >>>
> >>> Simon?
> >>
> >> Indeed, the new improved version is much better :)
> >>
> >> Acked-By: Simon Hausmann <simon@lst.de>
> >
> >Thanks.  This should go to 'maint' (part of v1.5.3.3) right?
>
> Sounds good by me.  I've been using it on a few other repos, and haven't
> had any problems.

Agreed.


Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: David Kastrup @ 2007-09-24  6:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhcllc9bz.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Ok, this is not really what we have been talking about except in
>> one case, but I think it is actually more of an improvement.
>
> Gaah, didn't I say I do NOT think it is an improvement?

Ah, but I am not presuming to speak for you in my commit message and
postings.

>> I consider breaking out of the condition instead of the
>> body od the loop ugly,
>
> Well, as we all know that we disagree on this point, stating
> what you consider one-sidedly here is quite inappropriate.

Hm.  If I create a patch after you basically said "go ahead, I don't
mind, but I consider it unimportant", how am I going to put the
motivation for the patch in the commit message while expressing _your_
opinion?  I thought that using "I" to make clear that it is my
personal view would be doing that.

So what am I supposed to write instead?

"There is no good reason for this patch, but we might as well do it."?

>> and the implied "true" value of the non-matching case is not really
>> obvious to humans at first glance.
>
> It is more like "if you do not know shell".

It is more to take in.  Believe me, I do know shell.

> In other words, I am somewhat disgusted with the first part of
> your proposed commit log message, although I like what the patch
> does ;-).

Could you propose a commit message that would be acceptable to you,
yet not make it appear like a mistake to actually commit the patch?

>> -while case "$#" in 0) break ;; esac
>> +while test "$#" != 0
>>  do
>>      case "$1" in
>>      -a)
>
> And let's not quote "$#".

I kept this as it was originally.  Some authors prefer to quote every
shell variable as a rule in order to avoid stupid syntactic things
happening.  Of course, $# never needs quoting, but I did not want to
change the personal style of the respective authors.  I can make this
consistent if you want to.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: David Kastrup @ 2007-09-24  6:26 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git
In-Reply-To: <20070924060521.GB10975@glandium.org>

Mike Hommey <mh@glandium.org> writes:

> On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
>> -while case $# in 0) break ;; esac
>> +while test $# != 0
>
> Wouldn't -ne be better ?

Why?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: David Symonds @ 2007-09-24  6:30 UTC (permalink / raw)
  To: David Kastrup; +Cc: Mike Hommey, git
In-Reply-To: <85k5qgk295.fsf@lola.goethe.zz>

On 24/09/2007, David Kastrup <dak@gnu.org> wrote:
> Mike Hommey <mh@glandium.org> writes:
>
> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
> >> -while case $# in 0) break ;; esac
> >> +while test $# != 0
> >
> > Wouldn't -ne be better ?
>
> Why?

Because -ne does a numeric comparison, != does a string comparison,
and it's a numeric comparison happening, semantically speaking.


Dave.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox