Wherefor worktrees?

[Marc Branchaud]

On 2018-09-26 11:48 AM, Duy Nguyen wrote:


I believe the main selling point of multiple worktrees is sharing
refs. You could easily avoid expensive clones with --local, but
synchronizing between different clones is not very convenient. Other
than that, different worktrees tend to behave like separate clones.


Sharing hooks is also useful, but yes mainly the refs.

I love being able to work in more than one branch at a time.  I often 
have a couple of ongoing big, messy topics, and being able to easily 
jump onto some release branch for a quick bugfix, without having to 
first stash things or finish an interactive rebase or fix a conflicting 
merge, is a godsend.


And the reason I use worktrees for this, instead of clones, is for the 
shared refs.  It makes sense to me that I'm working with different 
checkouts from a single repo, with all my local branches and local tags. 
 "git fetch" updates the remote refs regardless of which worktree I'm 
in when I run it.  The setup is lightweight and efficient; it's just how 
I want to work.


Having used git-new-workdir for a long time, it's main deficiency for me 
is submodules (the shared bisection state didn't bother me much).  It 
would be nice if all my worktrees' submodules also shared refs.  That's 
"nice", but not "essential".  Mainly it would be convenient if a 
recursive-submodule fetch performed in one worktree updated the 
submodule refs in my other worktrees.  Similarly, if I create a local 
branch in a submodule in one worktree, it would be nice to see that 
branch in the submodule in other worktrees.  Again, "nice", but probably 
just because I've lived with git-new-workdir's limitations for so long 
that I'm used to them.


That said, I really appreciate Duy's work here -- thanks!  Git deserves 
to have a cool feature like worktrees be part of its standard toolkit.


M.



This leaves a gray area where other things should be shared or not. I
think the preference (or default mode) is still _not_ shared (*).
Sharing more things (besides refs and object database) is just a new
opportunity popping up when we implement multiple worktrees. Since
multiple worktrees (or clones before its time) are grouped together,
sometimes you would like to share common configuration. We could sort
of achieve this already with includeIf but again not as convenient.

(*) real life is not that simple. Since refs are shared, including
_remotes_ refs, so configuration related to remotes should also be
shared, or it will be a maintenance nightmare. Which is why
$GIT_DIR/config so far has been shared besides the two exceptions that
are core.bare and core.worktree. And I really like to get rid of these
exceptions.


Is there a better way to achieve that without the
downside of multiple worktrees (e.g. configuration need to be
uniform)?


Is there a better way to achieve sharing refs between clones? I gave
it a minute but couldn't come up with a good answer :(


(*) "git config --worktree" points back to "config" file when this
 extension is not present so that it works in any setup.


Shouldn't it barf and error out instead?


The intention is a uniform interface/api that works with both single
and multiple worktrees configurations. Otherwise in your scripts you
would need to write "if single worktree, do this, else do that". If
"git config --worktree" works with both, the existing scripts can be
audited and updated just a bit, adding "--worktree" where the config
should not be shared, and we're done.


A user who hasn't enabled
the extension uses --worktree option and misled to believe that the
setting affects only a single worktree, even though the change is
made globally---that does not sound like a great end-user experience.


I was talking about a single worktree. But I think here you meant the
user has multiple worktrees, but the extension is not enabled. I'm
probably not clear in the commit message, but "git config" can detect
that the extension has not been enabled, automatically enable it (and
move core.bare and core.worktree (if present) to the main worktree's
private config), so "git config --worktree" will never share the
change.

But perhaps the user should be made aware of this situation and asked
to explicitly enable the extension instead? It's certainly a more
conservative approach.

[PATCH] fetch: Ensure that fetch.recurseSubmodules overrides submodule.recurse.

[Marc Branchaud]

Also document this fact.

Signed-off-by: Marc Branchaud 
---

I ran into this bug when I had both fetch.recurseSubmodules=on-demand and
submodule.recurse=true, and submodule.recurse was set *after*
fetch.recurseSubmodules in my config.

The fix ensures that fetch.recurseSubmodules always overrides
submodule.recurse.  If neither is set then fetch still behaves as if
fetch.recurseSubmodules=on-demand (the documented default).

I'm not sure if this is the most elegant implementation, but it gets the job
done.

M.


 Documentation/config.txt | 6 --
 builtin/fetch.c  | 5 -
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index eb66a11975..67b0adc1d4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1514,7 +1514,8 @@ fetch.recurseSubmodules::
recurse at all when set to false. When set to 'on-demand' (the default
value), fetch and pull will only recurse into a populated submodule
when its superproject retrieves a commit that updates the submodule's
-   reference.
+   reference.  This option overrides the more general submodule.recurse
+   option, for the `fetch` command.
 
 fetch.fsckObjects::
If it is set to true, git-fetch-pack will check all fetched
@@ -3465,7 +3466,8 @@ submodule.active::
 submodule.recurse::
Specifies if commands recurse into submodules by default. This
applies to all commands that have a `--recurse-submodules` option,
-   except `clone`.
+   except `clone`.  Also, the `fetch` command's behaviour can be specified
+   independently with the fetch.recurseSubmodules option.
Defaults to false.
 
 submodule.fetchJobs::
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 61bec5d213..08b8bf2741 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -60,6 +60,7 @@ static struct transport *gsecondary;
 static const char *submodule_prefix = "";
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int recurse_submodules_default = RECURSE_SUBMODULES_ON_DEMAND;
+static int recurse_submodules_set_explicitly = 0;
 static int shown_url = 0;
 static struct refspec refmap = REFSPEC_INIT_FETCH;
 static struct list_objects_filter_options filter_options;
@@ -78,7 +79,8 @@ static int git_fetch_config(const char *k, const char *v, 
void *cb)
return 0;
}
 
-   if (!strcmp(k, "submodule.recurse")) {
+   if (!strcmp(k, "submodule.recurse") &&
+   !recurse_submodules_set_explicitly) {
int r = git_config_bool(k, v) ?
RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
recurse_submodules = r;
@@ -88,6 +90,7 @@ static int git_fetch_config(const char *k, const char *v, 
void *cb)
max_children = parse_submodule_fetchjobs(k, v);
return 0;
} else if (!strcmp(k, "fetch.recursesubmodules")) {
+   recurse_submodules_set_explicitly = 1;
recurse_submodules = parse_fetch_recurse_submodules_arg(k, v);
return 0;
}
-- 
2.19.0.1.g5109f9487a

Re: Is rebase --force-rebase any different from rebase --no-ff?

[Marc Branchaud]

On 2018-05-09 03:46 PM, Ilya Kantor wrote:

I tried to compare --force-rebase VS --no-ff for the following repository:
http://jmp.sh/E7TRjcL

There's no difference in the resulf of:
git rebase --force-rebase 54a4
git rebase --no-ff 54a4

(rebases all 3 commits of feature)

Also, there's no difference in interactive mode:
git rebase --force-rebase -i 54a4
git rebase --no-ff -i 54a4

(picks all 3 commits of feature)

Is there a case where --no-ff differs from --force-rebase?


So now that "rebase -i" respects --force-rebase, the question is what to 
do about it:


1. Teach "rebase -i" to stop respecting --force-rebase (restoring the 
original intent when --no-ff was introduced)?


2. Deprecate --no-ff?

3. Deprecate --force-rebase?

As a heavy rebase user, I find --no-ff more intuitive than 
--force-rebase.  I'd be in favour of option 3, and keeping just --no-ff 
(with -f as a synonym).


M.



---
Best Regards,
Ilya Kantor


On Wed, May 9, 2018 at 10:27 PM, Marc Branchaud <marcn...@xiplink.com> wrote:

On 2018-05-09 02:21 PM, Stefan Beller wrote:


+cc Marc and Johannes who know more about rebase.

On Wed, May 9, 2018 at 9:01 AM, Ilya Kantor <ilia...@gmail.com> wrote:


Right now in "git help rebase" for --no-ff:
"Without --interactive, this is a synonym for --force-rebase."

But *with* --interactive, is there any difference?



I found

https://code.googlesource.com/git/+/b499549401cb2b1f6c30d09681380fd519938eb0
from 2010-03-24



In the original discussion around this option [1], at one point I proposed
teaching rebase--interactive to respect --force-rebase instead of adding a
new option [2].  Ultimately --no-ff was chosen as the better user interface
design [3], because an interactive rebase can't be "forced" to run.

At the time, I think rebase--interactive only recognized --no-ff.  That
might have been muddled a bit in the migration to rebase--helper.c.

Looking at it now, I don't have a strong opinion about keeping both options
or deprecating one of them.

 M.

[1] https://public-inbox.org/git/4b9fd9c1.9060...@xiplink.com/t/
[2]
https://public-inbox.org/git/1269361187-31291-1-git-send-email-marcn...@xiplink.com/
[3] https://public-inbox.org/git/7vzl1yd5j4@alter.siamese.dyndns.org/



  Teach rebase the --no-ff option.

  For git-rebase.sh, --no-ff is a synonym for --force-rebase.

  For git-rebase--interactive.sh, --no-ff cherry-picks all the commits
in
  the rebased branch, instead of fast-forwarding over any unchanged
commits.

  --no-ff offers an alternative way to deal with reverted merges.
Instead of
  "reverting the revert" you can use "rebase --no-ff" to recreate the
branch
  with entirely new commits (they're new because at the very least the
  committer time is different).  This obviates the need to revert the
  reversion, as you can re-merge the new topic branch directly.  Added
an
  addendum to revert-a-faulty-merge.txt describing the situation and
how to
  use --no-ff to handle it.

which sounds as if there is?

Stefan

Re: Is rebase --force-rebase any different from rebase --no-ff?

[Marc Branchaud]

On 2018-05-09 02:21 PM, Stefan Beller wrote:

+cc Marc and Johannes who know more about rebase.

On Wed, May 9, 2018 at 9:01 AM, Ilya Kantor  wrote:

Right now in "git help rebase" for --no-ff:
"Without --interactive, this is a synonym for --force-rebase."

But *with* --interactive, is there any difference?


I found
https://code.googlesource.com/git/+/b499549401cb2b1f6c30d09681380fd519938eb0
from 2010-03-24


In the original discussion around this option [1], at one point I 
proposed teaching rebase--interactive to respect --force-rebase instead 
of adding a new option [2].  Ultimately --no-ff was chosen as the better 
user interface design [3], because an interactive rebase can't be 
"forced" to run.


At the time, I think rebase--interactive only recognized --no-ff.  That 
might have been muddled a bit in the migration to rebase--helper.c.


Looking at it now, I don't have a strong opinion about keeping both 
options or deprecating one of them.


M.

[1] https://public-inbox.org/git/4b9fd9c1.9060...@xiplink.com/t/
[2] 
https://public-inbox.org/git/1269361187-31291-1-git-send-email-marcn...@xiplink.com/

[3] https://public-inbox.org/git/7vzl1yd5j4@alter.siamese.dyndns.org/



 Teach rebase the --no-ff option.

 For git-rebase.sh, --no-ff is a synonym for --force-rebase.

 For git-rebase--interactive.sh, --no-ff cherry-picks all the commits in
 the rebased branch, instead of fast-forwarding over any unchanged commits.

 --no-ff offers an alternative way to deal with reverted merges.  Instead of
 "reverting the revert" you can use "rebase --no-ff" to recreate the branch
 with entirely new commits (they're new because at the very least the
 committer time is different).  This obviates the need to revert the
 reversion, as you can re-merge the new topic branch directly.  Added an
 addendum to revert-a-faulty-merge.txt describing the situation and how to
 use --no-ff to handle it.

which sounds as if there is?

Stefan

Re: [RFC PATCH] checkout: Force matching mtime between files

[Marc Branchaud]

On 2018-04-27 01:03 PM, Duy Nguyen wrote:

On Wed, Apr 25, 2018 at 5:18 PM, Marc Branchaud <marcn...@xiplink.com> wrote:

The best approach to do so is to have those people do the "touch"
thing in their own post-checkout hook.  People who use Git as the
source control system won't have to pay runtime cost of doing the
touch thing, and we do not have to maintain such a hook script.
Only those who use the "feature" would.



The post-checkout hook approach is not exactly straightforward.


I am revisiting this because I'm not even happy with my
post-checkout-modified hook suggestion, so..



Naively, it's simply

 for F in `git diff --name-only $1 $2`; do touch "$F"; done

But consider:

* Symlinks can cause the wrong file to be touched.  (Granted, Michał's
proposed patch also doesn't deal with symlinks.)  Let's assume that a hook
can be crafted will all possible sophistication.  There are still some
fundamental problems:


OK so this one could be tricky to get right, but it's possible.



* In a "file checkout" ("git checkout -- path/to/file"), $1 and $2 are
identical so the above loop does nothing.  Offhand I'm not even sure how a
hook might get the right files in this case.


This is a limitation of the current post-checkout hook. $3==0 from the
hook lets us know this is not a branch switch, but it does not really
tell you the affected paths. If it somehow passes all the given
pathspec to you, then you should be able to do "git ls-files --
$pathspec" which gives you the exact same set of paths that
git-checkout updates. We could do this by setting $4 to "--" and put
all the pathspecs in $5+ [1] e.g. "HEAD@{1} HEAD 0 -- path/to/file" in
the above example.

There is  third case here, if you do "git checkout  --
path/to/file" then it cannot be covered by the current design. I guess
we could set $3 to '2' (retrieve from a tree) to indicate this in
addition to 0 (from index) and 1 (from switching branch) and then $1
could be the tree in question (pathspecs are passed the same way
above)

[1] I wonder if we could have a more generic approach to pass
pathspecs via environment, which could work for more than just this
one hook. Not sure if it's a good idea though.


I think there needs to be something other than listing all the paths in 
the command is viable, because it's too easy to hit some 
command-line-length limit.  It would also be good if hook authors didn't 
have to re-invent the wheel of determining the changed paths for every 
corner-case.


My first instinct is to write them one-per-line on the hook's stdin. 
That's probably not generic enough for most hooks, but it seems like a 
good approach for this proposal.


Throwing them into a temporary file with a known name is also good --- 
better, I think, than stuffing them into an environment variable.


M.


* The hook has to be set up in every repo and submodule (at least until
something like Ævar's experiments come to fruition).


Either --template or core.hooksPath would solve this, or I'll try to
get my "hooks.* config" patch in. I think that one is a good thing to
do anyway because it allows more flexible hook management (and it
could allow multiple hooks of the same name too). With this, we could
avoid adding more command-specific config like core.fsmonitor or
uploadpack.packObjectsHook which to me are hooks.

Another option is extend core.hooksPath for searching multiple places
instead of just one like AEvar suggested.





* A fresh clone can't run the hook.  This is especially important when
dealing with submodules.  (In one case where we were bit by this, make
though that half of a fresh submodule clone's files were stale, and decided
to re-autoconf the entire thing.)


Both --template and config-based hooks should let you handle this case.

So, I think if we improve the hook system to give more information
(which is something we definitely should do), we could do this without
adding special cases in git. I'm not saying that we should never add
special cases, but at least this lets us play with different kinds of
post-checkout activities before we decide which one would be best
implemented in git.

Re: [RFC PATCH] checkout: Force matching mtime between files

[Marc Branchaud]

On 2018-04-25 09:25 PM, Junio C Hamano wrote:

Marc Branchaud <marcn...@xiplink.com> writes:


But Git is not an archiver (tar), but is a source code control
system, so I do not think we should spend any extra cycles to
"improve" its behaviour wrt the relative ordering, at least for the
default case.  Only those who rely on having build artifact *and*
source should pay the runtime (and preferrably also the
maintainance) cost.


Anyone who uses "make" or some other mtime-based tool is affected by
this.  I agree that it's not "Everyone" but it sure is a lot of
people.


That's an exaggerated misrepresentation.  Only those who put build
artifacts as well as source to SCM *AND* depend on mtime are
affected.

A shipped tarball often contain configure.in as well as generated
configure, so that consumers can just say ./configure without having
the whole autoconf toolchain to regenerate it (I also heard horror
stories that this is done to control the exact version of autoconf
to avoid compatibility issues), but do people arrange configure to
be regenerated from configure.in in their Makefile of such a project
automatically when building the default target?


Yes.  I've seen many automake-generated Makefiles with "recheck" 
machinery that'll conveniently rerun autoconf if "necessary".


(And those horror stories are true, BTW.)


In any case, that is
a tarball usecase, not a SCM one.


No, it's an SCM case when you need to have the project's code as part of 
your own.  I can't make everyone do things the Right Way, so I'm stuck 
using projects that are not 100% pure-source, because they "helpfully" 
release their code after the autoconf step for some reason.



Are we all that sure that the performance hit is that drastic?  After
all, we've just done write_entry().  Calling utime() at that point
should just hit the filesystem cache.


I do not know about others, but I personally am more disburbed by
the conceptual ugliness that comes from having to have such a piece
of code in the codebase.


The ugliness arises from the problem being solved.  It's not git's fault 
that the world is so stupid.


That git might be willing to suffer a bit of self-mutilation for the 
benefit of its users should be seen as a point of pride.


M.

Re: [RFC PATCH] checkout: Force matching mtime between files

[Marc Branchaud]

On 2018-04-25 04:48 AM, Junio C Hamano wrote:

"Robin H. Johnson"  writes:


In the thread from 6 years ago, you asked about tar's behavior for
mtimes. 'tar xf' restores mtimes from the tar archive, so relative
ordering after restore would be the same, and would only rebuild if the
original source happened to be dirty.

This behavior is already non-deterministic in Git, and would be improved
by the patch.


But Git is not an archiver (tar), but is a source code control
system, so I do not think we should spend any extra cycles to
"improve" its behaviour wrt the relative ordering, at least for the
default case.  Only those who rely on having build artifact *and*
source should pay the runtime (and preferrably also the
maintainance) cost.


Anyone who uses "make" or some other mtime-based tool is affected by 
this.  I agree that it's not "Everyone" but it sure is a lot of people.


Are we all that sure that the performance hit is that drastic?  After 
all, we've just done write_entry().  Calling utime() at that point 
should just hit the filesystem cache.



The best approach to do so is to have those people do the "touch"
thing in their own post-checkout hook.  People who use Git as the
source control system won't have to pay runtime cost of doing the
touch thing, and we do not have to maintain such a hook script.
Only those who use the "feature" would.


The post-checkout hook approach is not exactly straightforward.

Naively, it's simply

for F in `git diff --name-only $1 $2`; do touch "$F"; done

But consider:

* Symlinks can cause the wrong file to be touched.  (Granted, Michał's 
proposed patch also doesn't deal with symlinks.)  Let's assume that a 
hook can be crafted will all possible sophistication.  There are still 
some fundamental problems:


* In a "file checkout" ("git checkout -- path/to/file"), $1 and $2 are 
identical so the above loop does nothing.  Offhand I'm not even sure how 
a hook might get the right files in this case.


* The hook has to be set up in every repo and submodule (at least until 
something like Ævar's experiments come to fruition).


* A fresh clone can't run the hook.  This is especially important when 
dealing with submodules.  (In one case where we were bit by this, make 
though that half of a fresh submodule clone's files were stale, and 
decided to re-autoconf the entire thing.)



I just don't think the hook approach can completely solve the problem.

I appreciate Ævar's concern that there are more than just two mtime 
requests floating around.  But I think git's users are best served by a 
built-in approach, with a config setting to control the desired mtime 
handling (defaulting to the current behaviour).  People who want a 
different mtime solution will at least have a clear place in the code to 
propose a patch.


M.

Re: [RFC PATCH] checkout: Force matching mtime between files

[Marc Branchaud]

On 2018-04-13 01:01 PM, Michał Górny wrote:

Currently git does not control mtimes of files being checked out.  This
means that the only assumption you could make is that all files created
or modified within a single checkout action will have mtime between
start time and end time of this checkout.  The relations between mtimes
of different files depend on the order in which they are checked out,
filesystem speed and timestamp precision.


Thanks for scratching this old itch [1]!

Big +1 from me.  We've had incremental smoke-test rebuilds fail because 
of inconsistent file timestamps.


M.

[1] https://public-inbox.org/git/50c79d1f.1080...@xiplink.com/


Git repositories sometimes contain both generated and respective source
files.  For example, the Gentoo 'user syncing' repository combines
source ebuild files with generated metadata cache for user convenience.
Ideally, the 'git checkout' would be fast enough that (combined with low
timestamp precision) all files created or modified within a single
checkout would have matching timestamp.  However, in reality the cache
files may end up being wrongly 'older' than source file, requiring
unnecessary recheck.

The opposite problem (cache files not being regenerated when they should
have been) might also occur.  However, it could not be solved without
preserving timestamps, therefore it is out of scope of this change.

In order to avoid unnecessary cache mismatches, force a matching mtime
between all files created by a single checkout action.  This seems to be
the best course of action.  Matching mtimes do not trigger cache
updates.  They also match the concept of 'checkout' being an atomic
action.  Finally, this change does not break backwards compatibility
as the new result is a subset of the possible previous results.

For example, let's say that 'git checkout' creates two files in order,
with respective timestamps T1 and T2.  Before this patch, T1 <= T2.
After this patch, T1 == T2 which also satisfies T1 <= T2.

A similar problem was previously being addressed via git-restore-mtime
tool.  However, that solution is unnecessarily complex for Gentoo's use
case and does not seem to be suitable for 'seamless' integration.

The patch integrates mtime forcing via adding an additional member of
'struct checkout'.  This seemed the simplest way of adding it without
having to modify prototypes and adjust multiple call sites.  The member
is set to the current time in check_updates() function and is afterwards
enforced by checkout_entry().

The patch uses utime() rather than futimes() as that seems to be
the function used everywhere else in git and provided some MinGW
compatibility code.

Signed-off-by: Michał Górny 
---
  cache.h|  1 +
  entry.c| 12 +++-
  unpack-trees.c |  1 +
  3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index bbaf5c349..9f0a7c867 100644
--- a/cache.h
+++ b/cache.h
@@ -1526,6 +1526,7 @@ struct checkout {
const char *base_dir;
int base_dir_len;
struct delayed_checkout *delayed_checkout;
+   time_t checkout_mtime;
unsigned force:1,
 quiet:1,
 not_new:1,
diff --git a/entry.c b/entry.c
index 2101201a1..7ee5a6d28 100644
--- a/entry.c
+++ b/entry.c
@@ -411,6 +411,7 @@ int checkout_entry(struct cache_entry *ce,
  {
static struct strbuf path = STRBUF_INIT;
struct stat st;
+   int ret;
  
  	if (topath)

return write_entry(ce, topath, state, 1);
@@ -473,5 +474,14 @@ int checkout_entry(struct cache_entry *ce,
return 0;
  
  	create_directories(path.buf, path.len, state);

-   return write_entry(ce, path.buf, state, 0);
+   ret = write_entry(ce, path.buf, state, 0);
+
+   if (ret == 0 && state->checkout_mtime != 0) {
+   struct utimbuf t;
+   t.modtime = state->checkout_mtime;
+   if (utime(path.buf, ) < 0)
+   warning_errno("failed utime() on %s", path.buf);
+   }
+
+   return ret;
  }
diff --git a/unpack-trees.c b/unpack-trees.c
index e73745051..e1efefb68 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -346,6 +346,7 @@ static int check_updates(struct unpack_trees_options *o)
state.quiet = 1;
state.refresh_cache = 1;
state.istate = index;
+   state.checkout_mtime = time(NULL);
  
  	progress = get_progress(o);
  

Re: [PATCH V2] config: add --expiry-date

[Marc Branchaud]

On 2017-11-14 01:21 AM, Christian Couder wrote:

On Tue, Nov 14, 2017 at 3:04 AM,   wrote:

From: Haaris 

Description:
This patch adds a new option to the config command.

Uses flag --expiry-date as a data-type to covert date-strings to
timestamps when reading from config files (GET).
This flag is ignored on write (SET) because the date-string is stored in
config without performing any normalization.

Creates a few test cases and documentation since its a new feature.

Motivation:
A parse_expiry_date() function already existed for api calls,
this patch simply allows the function to be used from the command line.

Signed-off-by: Haaris 


Documentation/SubmittingPatches contains the following:

"Also notice that a real name is used in the Signed-off-by: line. Please
don't hide your real name."

And there is the following example before that:

 Signed-off-by: Random J Developer 

So it looks like "a real name" actually means "a real firstname and a
real surname".

It would be nice if your "Signed-off-by:" could match this format.


It might already match that format if Haaris lives in a society that 
only uses single names.


Still, such names are unusual enough that it's good to check that new 
contributors are following the guidelines properly.


M.



Also if you have a "From:" line at the beginning of the patch, please
make sure that the name there is tha same as on the "Signed-off-by:".

Thanks for working on this,
Christian.

Re: Recovering from gc errors

[Marc Branchaud]

(It turned out that this problem is related to worktrees.  CCing some 
worktree folks.)


On 2017-11-14 12:53 AM, Jeff King wrote:

On Mon, Nov 13, 2017 at 04:13:19PM -0500, Marc Branchaud wrote:


Various incantations of "git show ... 9c355a7726e31" only fail with the same
error, so I can't determine much about the problematic commit. Luckily I'm
not particularly concerned with losing objects, as I push any important
progress to named refs in backup repos.


Doing "git show" will require looking at the parent commit to produce
the diff. Probably "git show -s" would work. But in general for poking
at corruption, something bare-bones like "git cat-file commit 9c355a77"
is going to be your best bet.


Thanks, I'd forgotten about cat-file (show's -s did not work).

Only one or two of the bad commits could possibly belong in a submodule, 
so I don't think I'm seeing a worktree+submodule problem.


There are some definite "rebase -i" commits (e.g. "fixup!"), and a lot 
of what were probably cherry-picks.  I know I did these operations in a 
worktree (see below).



But I would like to clean this up in my local repo so that gc stops failing.
I tried simply removing this and other loose commits that trip up gc (i.e.
the objects/9c/355a7726e31b3033b8e714cf7edb4f0a41d8d4 file -- there are 49
such files, all of which are several months old), but now gc complains of a
bad tree object:


You can't generally fix corruption issues by deleting objects[1]. The
"source" that makes Git want to have these objects is the refs and
reflogs. So your best bet is to find which of those point to the
problematic objects and delete them.

I'd start by seeing if the breakage is reachable from any refs:

   git rev-list --objects --all >/dev/null


That command does succeed.


If that command succeeds, then all your refs are intact and the problem
is in the reflogs. You can try to figure out which, but I'd probably
just blow them all away:

   rm -rf .git/logs


Unfortunately, removing the logs directory does not fix "git gc".  So I 
restored it.


However I did find all of the bad SHAs in the HEAD logs of four of my 
worktrees.


All of those worktrees have directories in .git/worktrees/, but "git 
worktree list" does not show two of them.  "git worktree prune -v" 
displays and does nothing.  (I do not want to play with --expire, 
because I'd rather keep my other worktrees.)


I removed all of those worktrees' directories from .git/worktrees/, and 
now "git gc" succeeds.  I've also removed those worktrees' working 
directories, as I don't really need them anymore.


Thanks for your help!

I'm willing to chalk this up to bugs in the early worktree code, unless 
one of the CC'd worktree developers thinks otherwise.


An explicit "git worktree delete" command would be nice for manually 
cleaning things up.  It's easy to just delete the directory, but having 
a "delete" command gives the user assurance that they're not missing 
something.


M.


If the rev-list fails, then one or more branch is corrupted.
Unfortunately the usual efficient tools for asking "which branch
contains this object" are likely to be broken by the corruption. But you
can brute-force it, like:

   git for-each-ref --format='%(refname)' |
   while read ref; do
 git rev-list --objects "$ref" >/dev/null 2>&1 ||
 echo "$ref is broken"
   done

Hopefully that turns up only branches with little value, and you can
delete them:

   git update-ref -d $broken_ref

-Peff

[1] A note on my "you can't fix corruption by deleting objects".

 Since abcb86553d (pack-objects: match prune logic for discarding
 objects, 2014-10-15) , git-gc also traverses the history graph of
 unreachable but "recent" objects. This is to keep whole chunks of
 the history graph intact during the gc grace period (which is 2
 weeks by default). So object themselves _can_ be a source of
 traversal for git-gc.

 We do that traversal with the ignore_missing_links flag, so
 breakages in the unreachable objects _shouldn't_ cause what you're
 seeing. IIRC we did turn up a bug or two with ignore_missing_links.
 The only one I could find was a3ba6bf10a (revision.c: ignore broken
 tags with ignore_missing_links, 2017-05-20), which I think wouldn't
 generate the output you're seeing.

Recovering from gc errors

[Marc Branchaud]

(I'm using git 2.15.0.)

So today "git gc" started complaining:

error: Could not read 2bc277bcb7e9cc6ef2ea677dd1c3dcd1f9af0c2b
fatal: Failed to traverse parents of commit 
9c355a7726e31b3033b8e714cf7edb4f0a41d8d4

error: failed to run repack

I suspect I'm a victim of the worktree+submodule bugs -- as a longtime 
user of contrib/workdir/git-new-workdir, I've been playing with the 
"worktree" command since it was first introduced.  The "git gc" error 
occurs when it's run in my main repo; I have not tried it in any of my 
worktrees/workdirs.


Various incantations of "git show ... 9c355a7726e31" only fail with the 
same error, so I can't determine much about the problematic commit. 
Luckily I'm not particularly concerned with losing objects, as I push 
any important progress to named refs in backup repos.


But I would like to clean this up in my local repo so that gc stops 
failing.  I tried simply removing this and other loose commits that trip 
up gc (i.e. the objects/9c/355a7726e31b3033b8e714cf7edb4f0a41d8d4 file 
-- there are 49 such files, all of which are several months old), but 
now gc complains of a bad tree object:


error: Could not read c1a99c3520f0b456b8025c50302a4cc9b0b2d777
fatal: bad tree object c1a99c3520f0b456b8025c50302a4cc9b0b2d777
error: failed to run repack

This object is not lying around loose.  "git fsck" lists several 
dangling blob/commit/tree objects, but none of them are c1a99c3520f0b4.


So I'm not sure what to do next.

Any suggestions?

Thanks,

M.

Re: [RFC PATCH] proposal for refs/tracking/* hierarchy

[Marc Branchaud]

On 2017-06-23 04:54 PM, Junio C Hamano wrote:

Jacob Keller  writes:


Instead, lets add support for a new refs/tracking/* hierarchy which is
laid out in such a way to avoid this inconsistency. All refs in
"refs/tracking//*" will include the complete ref, such that
dropping the "tracking/" part will give the exact ref name as it
is found in the upstream. Thus, we can track any ref here by simply
fetching it into refs/tracking//*.


I do think this overall is a good goal to aim for wrt "tracking",
i.e.  keeping a pristine copy of what we observed from the outside
world.  In addition to what you listed as "undecided" below,
however, I expect that this will highlight a lot of trouble in
"working together", i.e. reconciling the differences of various
world views and moving the project forward, in two orthogonal axes:

  - Things pointed at by some refs (e.g. notes/, but I think the
".gitmodules equivalent that is not tied to any particular commit
in the superproject" Jonathan Nieder and friends advocate falls
into the same category) do not correspond to the working tree
contents, and merging their contents will always pose challenge
when we need to prepare for conflict resolution.


OTOH, we shouldn't need to do any conflict resolution on these 
"tracking" refs:  The remote side should update the refs properly. 
Nobody should make local changes to these refs.


I guess I'm advocating that git should only allow "tracking" refs to be 
updated by a fetch, or a successful push of a local, non-"tracking" ref.


M.



  - Things pointed at by some other refs (e.g. tags/) do not make
sense to be merged.  You may locally call a commit with a tag
"wip", while your friends may use the same "wip" tag to point at
a different one.  Both are valid, and more importantly, there is
no point even trying to reconcile what the "wip" tag means in the
project globally.

For the former, I expect that merging these non-working tree
contents will all have to require some specific tool that is
tailored for the meaning each hierarchy has, just like "git notes
merge" gives a solution that is very specific to the notes refs (I
do not know how well "notes merge" works in practice, though).

For the latter, having a separate tracking hierarchy is a strict
improvement from "tracking" point of view, but I think "working
together" also needs a well designed set of new look-up rules, and a
new convention.  For example, some tags may not want to be shared
(e.g. "wip" example above) even though they should be easy to access
by those who already have them (e.g. "git log ..wip" should work
without having to say "git log ..refs/local-tags/wip", even if we
decide to implement the "some tags may not want to be shared"
segregation by introducing a new hierarchy).  And also a shared tag
that is copied to "refs/tracking/origin/tags/v1.0" would need a way
better than "git log tracking/origin/tags/v1.0" for this mechanism
to be useful in everyday workflow.

Thanks.

[PATCHv2 (resend)] Tweak help auto-correct phrasing.

[Marc Branchaud]

When auto-correct is enabled, an invalid git command prints a warning and
a continuation message, which differs depending on whether or not
help.autoCorrect is positive or negative.

With help.autoCorrect = 15:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing under the assumption that you meant 'log'
   in 1.5 seconds automatically...

With help.autoCorrect < 0:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing under the assumption that you meant 'log'

The continuation message's phrasing is awkward.  This commit cleans it up.
As a bonus, we now use full-sentence strings which make translation easier.

With help.autoCorrect = 15:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing in 1.5 seconds, assuming that you meant 'log'.

With help.autoCorrect < 0:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing under the assumption that you meant 'log'.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---

So here's the patch again.

M.

 help.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/help.c b/help.c
index f637fc800..69966c174 100644
--- a/help.c
+++ b/help.c
@@ -356,12 +356,18 @@ const char *help_unknown_cmd(const char *cmd)
clean_cmdnames(_cmds);
fprintf_ln(stderr,
   _("WARNING: You called a Git command named '%s', "
-"which does not exist.\n"
-"Continuing under the assumption that you meant 
'%s'"),
-   cmd, assumed);
-   if (autocorrect > 0) {
-   fprintf_ln(stderr, _("in %0.1f seconds 
automatically..."),
-   (float)autocorrect/10.0);
+"which does not exist."),
+  cmd);
+   if (autocorrect < 0)
+   fprintf_ln(stderr,
+  _("Continuing under the assumption that "
+"you meant '%s'."),
+  assumed);
+   else {
+   fprintf_ln(stderr,
+  _("Continuing in %0.1f seconds, "
+"assuming that you meant '%s'."),
+  (float)autocorrect/10.0, assumed);
sleep_millisec(autocorrect * 100);
}
return assumed;
-- 
2.13.1.388.g69e6b9b4f.dirty

Re: [PATCHv2] Tweak help auto-correct phrasing.

[Marc Branchaud]

On 2017-06-20 02:04 PM, Kaartic Sivaraam wrote:

On Tue, 2016-12-20 at 09:02 -0500, Marc Branchaud wrote:

When auto-correct is enabled, an invalid git command prints a warning
and
a continuation message, which differs depending on whether or not
help.autoCorrect is positive or negative.

With help.autoCorrect = 15:

WARNING: You called a Git command named 'lgo', which does not
exist.
Continuing under the assumption that you meant 'log'
in 1.5 seconds automatically...

With help.autoCorrect < 0:

WARNING: You called a Git command named 'lgo', which does not
exist.
Continuing under the assumption that you meant 'log'

The continuation message's phrasing is awkward.  This commit cleans
it up.
As a bonus, we now use full-sentence strings which make translation
easier.

With help.autoCorrect = 15:

WARNING: You called a Git command named 'lgo', which does not
exist.
Continuing in 1.5 seconds, assuming that you meant 'log'.

With help.autoCorrect < 0:

WARNING: You called a Git command named 'lgo', which does not
exist.
Continuing under the assumption that you meant 'log'.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---


Excuse me for bringing this up after a long time. What's the status of
this patch? Was it applied?


Looks like it got lost in the shuffle.

The topic thread starts at:
http://public-inbox.org/git/1482063500.10858.1.ca...@gmail.com/

There's no reply to my v2 patch, and I neglected to follow up on it -- 
sorry!


Shall I resend the patch?

M.

Re: proposal for how to share other refs as part of refs/tracking/*

[Marc Branchaud]

On 2017-06-13 10:41 AM, Marc Branchaud wrote:


So I like your refs/tracking proposal, and hope that it aims for 
mirroring a remote's refs, to eventually replace refs/remotes entirely.


To be extra-clear:

I think a refs/tracking hierarchy that starts with notes and maybe some 
other bits is a good first step.


I *hope* such a first step can eventually mirror all of a remote's refs, 
including heads and tags.


I in no way think that this effort should begin by tackling heads and tags.

M.

Re: proposal for how to share other refs as part of refs/tracking/*

[Marc Branchaud]

On 2017-06-12 06:58 PM, Jacob Keller wrote:

Hi,

There's no actual code yet, (forgive me), but I've been thinking back
to a while ago about attempting to find a way to share things like
refs/notes, and similar refs which are usually not shared across a
remote.

By default, those refs are not propagated when you do a push or a
pull, and this makes using them in any project which has more then one
repository quite difficult.

I'm going to focus the discussion primarily on refs/notes as this is
what I am most interested in, but I think similar issues exist for
refs/grafts and refs/replace, and maybe other sources?


More formal support for custom ref namespaces would be a boon.  For 
example, we have our nightly builds create a "build/w.x.y-z" ref (we 
only want to tag official releases).  Sharing those refs is not hard, 
but a bit obscure.



For branches, we already have a system to share the status of remote
branches, called "refs/remotes//"

This hierarchy unfortunately does not keep space for non-branches,
because you can't simply add a "refs/remotes/notes/<>" or
"refs/remotes//notes" to this as it would be ambiguous.

Now, you might just decide to push the refs/notes directly, ie:

git push origin refs/notes/...:refs/notes/...

Unfortunately, this has problems because it leaves no standard way to
distinguish your local work from what is on the remote, so you can't
easily merge the remote work into yours.


There was a related discussion in the run-up to 1.8.0, about a "remote 
tag namespace" to support having different remotes with the same tag 
name for different objects.  See these messages and their surrounding 
threads:


http://public-inbox.org/git/AANLkTikeqsg+qJ0z4iQ6ZmKL=_HB8YX_z20L=dffa...@mail.gmail.com/

http://public-inbox.org/git/AANLkTi=yfwoaqmhhvlsb1_xmyoe9hhp2yb4h4tqzw...@mail.gmail.com/

http://public-inbox.org/git/201102020322.00171.jo...@herland.net/

The discussion explored, among other things, making 
refs/remotes/$remote/* a mirror of a remote's own refs/* hierarchy 
(well, within reason -- I think there are limits to what should be 
mirrored).


So I like your refs/tracking proposal, and hope that it aims for 
mirroring a remote's refs, to eventually replace refs/remotes entirely.


M.



For example, if Alice creates a new note and pushes it, then Bob
creates a different note on the same commit, he needs to be able to
merge Alice's changes into his note, just like one would do when two
people commit to the same branch.

Today, he must pull the remote notes into a separate location, (since
pulling directly into refs/notes will overwrite his work), and then
update, and then push.

Because this is not standardized, it becomes unwieldy to actually
perform on a day to day basis.

I propose that we add a new "refs/tracking" hierarchy which will be
used to track these type of refs

We could even (long term) migrate refs/remotes into refs/tracking
instead, or provide both with the refs/remotes being pointers or
something like that..

Essentially, refs/notes would be pulled into
refs/tracking//notes/* or something along these lines.

Then, git notes would be modified to be able to have commands to
"pull" and "push" notes which would fetch the remote, and then attempt
a merge into the local notes, while a push would update the remote.

I chose "tracking" because it sort of fits the concept and does not
include things like "remote-notes" or "remotes-v2" which are a bit
weird.

I'm posting this on the mailing list prior to having code because I
wanted to get a sense of how people felt about the question and
whether others still felt it was an issue.

Essentially the goal is to standardize how any refs namespace can be
shared in such a way that local copies can tell what the remote had at
a fetch time, in order to allow easier handling of conflicts between
local and remote changes, just like we do for branches (heads) but
generalized so that other refs namespaces can get the same ability to
handle conflicts.

Thanks,
Jake

Re: [PATCH] tag: duplicate mention of --contains should mention --no-contains

[Marc Branchaud]

On 2017-05-15 11:01 AM, Ævar Arnfjörð Bjarmason wrote:

On Mon, May 15, 2017 at 4:20 PM, Marc Branchaud <marcn...@xiplink.com> wrote:

On 2017-05-15 08:23 AM, Ævar Arnfjörð Bjarmason wrote:


Fix a duplicate mention of --contains in the SYNOPSIS to mention
--no-contains.

This fixes an error introduced in my commit ac3f5a3468 ("ref-filter:
add --no-contains option to tag/branch/for-each-ref", 2017-03-24).

Signed-off-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
---
 Documentation/git-tag.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index f8a0b787f4..1eb15afa1c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u ] [-f] [-m  | -F ]
 [ | ]
 'git tag' -d ...
-'git tag' [-n[]] -l [--contains ] [--contains ]
+'git tag' [-n[]] -l [--contains ] [--no-contains ]



I think

[--[no-]contains ]

is the usual pattern...


[--points-at ] [--column[=] | --no-column]
[--create-reflog] [--sort=] [--format=]
[--[no-]merged []] [...]



... like with --[no-]merged, there.

M.


Thanks for the feedback, this was discussed earlier in the series and
we decided on the current format I'm submitting here.

Saying --[no-]merged is the convention we use for options where the
two are mutually exclusive, as is the case with the --[no-]merged
options:

$ git tag --merged v2.12.0 --no-merged v2.13.0
error: option `no-merged' is incompatible with --merged
[...]

But in the case of --contains and --no-contains you can provide both:

$ git tag --contains v2.12.0 --no-contains v2.13.0 'v*'
v2.12.0
v2.12.1
v2.12.2
v2.12.3
v2.13.0-rc0
v2.13.0-rc1
v2.13.0-rc2

So they don't use that convention, since it would imply that they're
mutually exclusive, rather than both being optional.


Ah, thanks.  My mistake!

M.

Re: [PATCH] tag: duplicate mention of --contains should mention --no-contains

[Marc Branchaud]

On 2017-05-15 08:23 AM, Ævar Arnfjörð Bjarmason wrote:

Fix a duplicate mention of --contains in the SYNOPSIS to mention
--no-contains.

This fixes an error introduced in my commit ac3f5a3468 ("ref-filter:
add --no-contains option to tag/branch/for-each-ref", 2017-03-24).

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-tag.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index f8a0b787f4..1eb15afa1c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u ] [-f] [-m  | -F ]
 [ | ]
 'git tag' -d ...
-'git tag' [-n[]] -l [--contains ] [--contains ]
+'git tag' [-n[]] -l [--contains ] [--no-contains ]


I think

[--[no-]contains ]

is the usual pattern...


[--points-at ] [--column[=] | --no-column]
[--create-reflog] [--sort=] [--format=]
[--[no-]merged []] [...]


... like with --[no-]merged, there.

M.

Re: What's cooking in git.git (May 2017, #03; Wed, 10)

[Marc Branchaud]

On 2017-05-10 01:18 AM, Junio C Hamano wrote:


* mb/diff-default-to-indent-heuristics (2017-05-09) 4 commits
 - add--interactive: drop diff.indentHeuristic handling
 - diff: enable indent heuristic by default
 - diff: have the diff-* builtins configure diff before initializing revisions
 - diff: make the indent heuristic part of diff's basic configuration

 Make the "indent" heuristics the default in "diff" and diff.indentHeuristics
 configuration variable an escape hatch for those who do no want it.


Typo fixes:
s/heuristics/heuristic/  (both places)
s/do no want/do not want/


 Kicked out of next; it seems it is still getting review suggestions?


I believe v4 of this one is ready to cook.

The most salient aspect of the review discussion was about where to go 
after this topic is applied.  We also concluded that the topic deserves 
a release note about breaking patch ID backwards-compatibility.  I think 
such a note should mention rerere, so I would suggest the following:


The diff "indent" heuristic is now enabled by default.  This changes the 
patch IDs calculated by git-patch-id (and used by git-rerere and 
git-cherry), which could affect workflows that rely on 
previously-computed patch IDs.  The heuristic can be disabled by setting 
diff.indentHeuristic to false.


M.

Re: [PATCH v4 2/4] diff: have the diff-* builtins configure diff before initializing revisions

[Marc Branchaud]

On 2017-05-08 11:22 PM, Jeff King wrote:

On Mon, May 08, 2017 at 12:03:37PM -0400, Marc Branchaud wrote:


This matches how the diff Porcelain works.  It makes the plumbing commands
respect diff's configuration options, such as indentHeuristic, because
init_revisions() calls diff_setup() which fills in the diff_options struct.


I don't know if you want to note here that this is only _some_ options.
I.e., ones that we handle by copying via diff_setup(). Maybe it's
obvious from the description already (it's hard for me to tell because I
already know it either way :) ).


(shrug)  I'm fine with the way it is, but I'd also be OK with "respect 
some of diff's configuration options".


Junio, please feel free to reword the message if you like.  Or I can 
send out a v5, if that's easier for you.


M.

[PATCH v4 1/4] diff: make the indent heuristic part of diff's basic configuration

[Marc Branchaud]

This heuristic was originally introduced as an experimental feature,
and therefore part of the UI configuration.

But the user often sees diffs generated by plumbing commands like
diff-tree.  Moving the indent heuristic into diff's basic configuration
prepares the way for diff plumbing commands to respect the setting.

The heuristic itself merely makes the diffs more aesthetically
pleasing, without changing their correctness.  Scripts that rely on
the diff plumbing commands should not care whether or not the heuristic
is employed.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 diff.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 74283d900..b6e3ffe92 100644
--- a/diff.c
+++ b/diff.c
@@ -290,9 +290,6 @@ int git_diff_ui_config(const char *var, const char *value, 
void *cb)
return 0;
}
 
-   if (git_diff_heuristic_config(var, value, cb) < 0)
-   return -1;
-
if (!strcmp(var, "diff.wserrorhighlight")) {
int val = parse_ws_error_highlight(value);
if (val < 0)
@@ -351,6 +348,9 @@ int git_diff_basic_config(const char *var, const char 
*value, void *cb)
if (starts_with(var, "submodule."))
return parse_submodule_config_option(var, value);
 
+   if (git_diff_heuristic_config(var, value, cb) < 0)
+   return -1;
+
return git_default_config(var, value, cb);
 }
 
-- 
2.13.0.rc1.15.gf67d331ad

[PATCH v4 2/4] diff: have the diff-* builtins configure diff before initializing revisions

[Marc Branchaud]

This matches how the diff Porcelain works.  It makes the plumbing commands
respect diff's configuration options, such as indentHeuristic, because
init_revisions() calls diff_setup() which fills in the diff_options struct.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 builtin/diff-files.c   |  2 +-
 builtin/diff-index.c   |  2 +-
 builtin/diff-tree.c|  2 +-
 t/t4061-diff-indent.sh | 66 ++
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 15c61fd8d..a572da9d5 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -20,9 +20,9 @@ int cmd_diff_files(int argc, const char **argv, const char 
*prefix)
int result;
unsigned options = 0;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
precompose_argv(argc, argv);
 
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 1af373d00..f084826a2 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -17,9 +17,9 @@ int cmd_diff_index(int argc, const char **argv, const char 
*prefix)
int i;
int result;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
precompose_argv(argc, argv);
 
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 326f88b65..36a3a1976 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -105,9 +105,9 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
struct setup_revision_opt s_r_opt;
int read_stdin = 0;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(opt, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
opt->abbrev = 0;
opt->diff = 1;
opt->disable_stdin = 1;
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 556450609..13d3dc96a 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -213,4 +213,70 @@ test_expect_success 'blame: --no-indent-heuristic 
overrides config' '
compare_blame spaces-expect out-blame2
 '
 
+test_expect_success 'diff-tree: nice spaces with --indent-heuristic' '
+   git diff-tree --indent-heuristic -p old new -- spaces.txt 
>out-diff-tree-compacted &&
+   compare_diff spaces-compacted-expect out-diff-tree-compacted
+'
+
+test_expect_success 'diff-tree: nice spaces with diff.indentHeuristic' '
+   git -c diff.indentHeuristic=true diff-tree -p old new -- spaces.txt 
>out-diff-tree-compacted2 &&
+   compare_diff spaces-compacted-expect out-diff-tree-compacted2
+'
+
+test_expect_success 'diff-tree: --no-indent-heuristic overrides config' '
+   git -c diff.indentHeuristic=true diff-tree --no-indent-heuristic -p old 
new -- spaces.txt >out-diff-tree &&
+   compare_diff spaces-expect out-diff-tree
+'
+
+test_expect_success 'diff-index: nice spaces with --indent-heuristic' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git diff-index --indent-heuristic -p old -- spaces.txt 
>out-diff-index-compacted &&
+   compare_diff spaces-compacted-expect out-diff-index-compacted &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-index: nice spaces with diff.indentHeuristic' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git -c diff.indentHeuristic=true diff-index -p old -- spaces.txt 
>out-diff-index-compacted2 &&
+   compare_diff spaces-compacted-expect out-diff-index-compacted2 &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-index: --no-indent-heuristic overrides config' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git -c diff.indentHeuristic=true diff-index --no-indent-heuristic -p 
old -- spaces.txt >out-diff-index &&
+   compare_diff spaces-expect out-diff-index &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-files: nice spaces with diff.indentHeuristic' '
+   git checkout -B diff-files &&
+   git reset HEAD~ &&
+   git -c diff.indentHeuristic=true diff-files -p spaces.txt 
>out-diff-files-raw &&
+   grep -v index out-diff-files-raw >out-diff-files-compacted &&
+   compare_diff spaces-compacted-expect out-diff-files-compacted &&
+   git checkout -f master
+'
+
+test_expect_

[PATCH v4 3/4] diff: enable indent heuristic by default

[Marc Branchaud]

From: Stefan Beller <sbel...@google.com>

The feature was included in v2.11 (released 2016-11-29) and we got no
negative feedback. Quite the opposite, all feedback we got was positive.

Turn it on by default. Users who dislike the feature can turn it off
by setting diff.indentHeuristic (which also configures plumbing commands,
see prior patches).

The change to t/t4051-diff-function-context.sh is needed because the
heuristic shifts the changed hunk in the patch.  To get the same result
regardless of the heuristic configuration, we modify the test file
differently:  We insert a completely new line after line 2, instead of
simply duplicating it.

Helped-by: Jeff King <p...@peff.net>
Signed-off-by: Stefan Beller <sbel...@google.com>
Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---

 diff.c   |   2 +-
 t/t4051-diff-function-context.sh |   3 +-
 t/t4061-diff-indent.sh   | 140 +++
 3 files changed, 116 insertions(+), 29 deletions(-)

diff --git a/diff.c b/diff.c
index b6e3ffe92..a24452051 100644
--- a/diff.c
+++ b/diff.c
@@ -27,7 +27,7 @@
 #endif
 
 static int diff_detect_rename_default;
-static int diff_indent_heuristic; /* experimental */
+static int diff_indent_heuristic = 1;
 static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
 static int diff_use_color_default = -1;
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 6154acb45..3e6b485ec 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -72,7 +72,8 @@ test_expect_success 'setup' '
 
# overlap function context of 1st change and -u context of 2nd change
grep -v "delete me from hello" <"$dir/hello.c" >file.c &&
-   sed 2p <"$dir/dummy.c" >>file.c &&
+   sed "2a\\
+extra line" <"$dir/dummy.c" >>file.c &&
commit_and_tag changed_hello_dummy file.c &&
 
git checkout initial &&
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 13d3dc96a..2affd7a10 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -152,26 +152,28 @@ test_expect_success 'prepare' '
EOF
 '
 
+# --- diff tests --
+
 test_expect_success 'diff: ugly spaces' '
-   git diff old new -- spaces.txt >out &&
+   git diff --no-indent-heuristic old new -- spaces.txt >out &&
compare_diff spaces-expect out
 '
 
+test_expect_success 'diff: --no-indent-heuristic overrides config' '
+   git -c diff.indentHeuristic=true diff --no-indent-heuristic old new -- 
spaces.txt >out2 &&
+   compare_diff spaces-expect out2
+'
+
 test_expect_success 'diff: nice spaces with --indent-heuristic' '
-   git diff --indent-heuristic old new -- spaces.txt >out-compacted &&
+   git -c diff.indentHeuristic=false diff --indent-heuristic old new -- 
spaces.txt >out-compacted &&
compare_diff spaces-compacted-expect out-compacted
 '
 
-test_expect_success 'diff: nice spaces with diff.indentHeuristic' '
+test_expect_success 'diff: nice spaces with diff.indentHeuristic=true' '
git -c diff.indentHeuristic=true diff old new -- spaces.txt 
>out-compacted2 &&
compare_diff spaces-compacted-expect out-compacted2
 '
 
-test_expect_success 'diff: --no-indent-heuristic overrides config' '
-   git -c diff.indentHeuristic=true diff --no-indent-heuristic old new -- 
spaces.txt >out2 &&
-   compare_diff spaces-expect out2
-'
-
 test_expect_success 'diff: --indent-heuristic with --patience' '
git diff --indent-heuristic --patience old new -- spaces.txt 
>out-compacted3 &&
compare_diff spaces-compacted-expect out-compacted3
@@ -183,7 +185,7 @@ test_expect_success 'diff: --indent-heuristic with 
--histogram' '
 '
 
 test_expect_success 'diff: ugly functions' '
-   git diff old new -- functions.c >out &&
+   git diff --no-indent-heuristic old new -- functions.c >out &&
compare_diff functions-expect out
 '
 
@@ -192,42 +194,73 @@ test_expect_success 'diff: nice functions with 
--indent-heuristic' '
compare_diff functions-compacted-expect out-compacted
 '
 
-test_expect_success 'blame: ugly spaces' '
-   git blame old..new -- spaces.txt >out-blame &&
-   compare_blame spaces-expect out-blame
-'
+# --- blame tests -
 
 test_expect_success 'blame: nice spaces with --indent-heuristic' '
git blame --indent-heuristic old..new -- spaces.txt 
>out-blame-compacted &&
compare_blame spaces-compacted-expect out-blame-compacted
 '
 
-test_expect_success 'blame: nice spaces with diff.indentHeuristic' '

[PATCH v4 4/4] add--interactive: drop diff.indentHeuristic handling

[Marc Branchaud]

From: Jeff King <p...@peff.net>

Now that diff.indentHeuristic is handled automatically by the plumbing
commands, there's no need to propagate it manually.

Signed-off-by: Jeff King <p...@peff.net>
Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 git-add--interactive.perl | 4 
 1 file changed, 4 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 709a5f6ce..79d675b5b 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -46,7 +46,6 @@ my ($diff_new_color) =
 my $normal_color = $repo->get_color("", "reset");
 
 my $diff_algorithm = $repo->config('diff.algorithm');
-my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');
 my $diff_filter = $repo->config('interactive.difffilter');
 
 my $use_readkey = 0;
@@ -730,9 +729,6 @@ sub parse_diff {
if (defined $diff_algorithm) {
splice @diff_cmd, 1, 0, "--diff-algorithm=${diff_algorithm}";
}
-   if ($diff_indent_heuristic) {
-   splice @diff_cmd, 1, 0, "--indent-heuristic";
-   }
if (defined $patch_mode_revision) {
push @diff_cmd, get_diff_reference($patch_mode_revision);
}
-- 
2.13.0.rc1.15.gf67d331ad

[PATCH v4 0/4] Make diff plumbing commands respect the indentHeuristic.

[Marc Branchaud]

The only change from v3 is in 3/4, to expand t4061 to test various
combinations of --(no-)indent-heuristic and diff.indentHeuristic.

I kindof went all-in and tried to cover every possible combination for
all four affected commands.

An inter-diff is below.

M.

Jeff King (1):
  add--interactive: drop diff.indentHeuristic handling

Marc Branchaud (2):
  diff: make the indent heuristic part of diff's basic configuration
  diff: have the diff-* builtins configure diff before initializing
revisions

Stefan Beller (1):
  diff: enable indent heuristic by default

 builtin/diff-files.c |   2 +-
 builtin/diff-index.c |   2 +-
 builtin/diff-tree.c  |   2 +-
 diff.c   |   8 +-
 git-add--interactive.perl|   4 -
 t/t4051-diff-function-context.sh |   3 +-
 t/t4061-diff-indent.sh   | 184 +++
 7 files changed, 177 insertions(+), 28 deletions(-)


diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 56d7d7760..2affd7a10 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -152,26 +152,28 @@ test_expect_success 'prepare' '
EOF
 '
 
+# --- diff tests --
+
 test_expect_success 'diff: ugly spaces' '
git diff --no-indent-heuristic old new -- spaces.txt >out &&
compare_diff spaces-expect out
 '
 
+test_expect_success 'diff: --no-indent-heuristic overrides config' '
+   git -c diff.indentHeuristic=true diff --no-indent-heuristic old new -- 
spaces.txt >out2 &&
+   compare_diff spaces-expect out2
+'
+
 test_expect_success 'diff: nice spaces with --indent-heuristic' '
-   git diff --indent-heuristic old new -- spaces.txt >out-compacted &&
+   git -c diff.indentHeuristic=false diff --indent-heuristic old new -- 
spaces.txt >out-compacted &&
compare_diff spaces-compacted-expect out-compacted
 '
 
-test_expect_success 'diff: nice spaces with diff.indentHeuristic' '
+test_expect_success 'diff: nice spaces with diff.indentHeuristic=true' '
git -c diff.indentHeuristic=true diff old new -- spaces.txt 
>out-compacted2 &&
compare_diff spaces-compacted-expect out-compacted2
 '
 
-test_expect_success 'diff: --no-indent-heuristic overrides config' '
-   git -c diff.indentHeuristic=true diff --no-indent-heuristic old new -- 
spaces.txt >out2 &&
-   compare_diff spaces-expect out2
-'
-
 test_expect_success 'diff: --indent-heuristic with --patience' '
git diff --indent-heuristic --patience old new -- spaces.txt 
>out-compacted3 &&
compare_diff spaces-compacted-expect out-compacted3
@@ -192,42 +194,73 @@ test_expect_success 'diff: nice functions with 
--indent-heuristic' '
compare_diff functions-compacted-expect out-compacted
 '
 
-test_expect_success 'blame: ugly spaces' '
-   git blame --no-indent-heuristic old..new -- spaces.txt >out-blame &&
-   compare_blame spaces-expect out-blame
-'
+# --- blame tests -
 
 test_expect_success 'blame: nice spaces with --indent-heuristic' '
git blame --indent-heuristic old..new -- spaces.txt 
>out-blame-compacted &&
compare_blame spaces-compacted-expect out-blame-compacted
 '
 
-test_expect_success 'blame: nice spaces with diff.indentHeuristic' '
+test_expect_success 'blame: nice spaces with diff.indentHeuristic=true' '
git -c diff.indentHeuristic=true blame old..new -- spaces.txt 
>out-blame-compacted2 &&
compare_blame spaces-compacted-expect out-blame-compacted2
 '
 
+test_expect_success 'blame: ugly spaces with --no-indent-heuristic' '
+   git blame --no-indent-heuristic old..new -- spaces.txt >out-blame &&
+   compare_blame spaces-expect out-blame
+'
+
+test_expect_success 'blame: ugly spaces with diff.indentHeuristic=false' '
+   git -c diff.indentHeuristic=false blame old..new -- spaces.txt 
>out-blame2 &&
+   compare_blame spaces-expect out-blame2
+'
+
 test_expect_success 'blame: --no-indent-heuristic overrides config' '
-   git -c diff.indentHeuristic=true blame --no-indent-heuristic old..new 
-- spaces.txt >out-blame2 &&
+   git -c diff.indentHeuristic=true blame --no-indent-heuristic old..new 
-- spaces.txt >out-blame3 &&
git blame old..new -- spaces.txt >out-blame &&
-   compare_blame spaces-expect out-blame2
+   compare_blame spaces-expect out-blame3
 '
 
+test_expect_success 'blame: --indent-heuristic overrides config' '
+   git -c diff.indentHeuristic=false blame --indent-heuristic old..new -- 
spaces.txt >out-blame-compacted3 &&
+   compare_blame spaces-compacted-expect out-blame-compacted2
+'
+
+# --- diff-tree tests ---

Enabling the diff "indent" heuristic by default

[Marc Branchaud]

On 2017-05-08 03:48 AM, Junio C Hamano wrote:


* mb/diff-default-to-indent-heuristics (2017-05-02) 4 commits
  (merged to 'next' on 2017-05-08 at 158f401a92)


I think there's a general open question about this, which is whether or 
not we should just drop the diff.indentHeuristic configuration setting 
altogether.


Peff made the point [0] that if we keep the setting then t4061 should be 
rewritten.


My instinct is to keep the setting, at least until the changed default 
has a bit of time to settle in.  So I'll re-send the topic with the 
renovated t4061.


The topic would of course change more drastically if we decide to drop 
the setting right away.



 + add--interactive: drop diff.indentHeuristic handling
 + diff: enable indent heuristic by default
 + diff: have the diff-* builtins configure diff before initializing revisions
 + diff: make the indent heuristic part of diff's basic configuration

 Make the "indent" heuristics the default in "diff" and diff.indentHeuristics


s/heuristics/heuristic/  (both places)


 configuration variable an escape hatch for those who do no want it.


s/do no/do not/


 Will cook in 'next'.


Both Peff [1] and Ævar [2] mentioned situations where enabling the 
heuristic has a small impact on them.  If/when this graduates, it's 
perhaps worth adding a backward-compatibility note that the default 
patch IDs are changing.  Maybe something like:


The diff "indent" heuristic is now enabled by default.  This changes the 
patch IDs calculated by git-patch-id and used by git-cherry, which could 
affect patch-based workflows that rely on previously-computed patch IDs. 
 The heuristic can be disabled by setting diff.indentHeuristic to false.


[0] 
https://public-inbox.org/git/20170501222051.svylxazjwnot3...@sigill.intra.peff.net/


[1] 
https://public-inbox.org/git/20170428220450.olqitnuwhrxzg...@sigill.intra.peff.net/


[2] 
https://public-inbox.org/git/cacbzzx5f81hkcjrjtdyxznmvuef9z_ecs+0svk2xpbwxudg...@mail.gmail.com/


M.

[PATCHv3 3/4] diff: enable indent heuristic by default

[Marc Branchaud]

From: Stefan Beller <sbel...@google.com>

The feature was included in v2.11 (released 2016-11-29) and we got no
negative feedback. Quite the opposite, all feedback we got was positive.

Turn it on by default. Users who dislike the feature can turn it off
by setting diff.indentHeuristic (which also configures plumbing commands,
see prior patches).

The change to t/t4051-diff-function-context.sh is needed because the
heuristic shifts the changed hunk in the patch.  To get the same result
regardless of the heuristic configuration, we modify the test file
differently:  We insert a completely new line after line 2, instead of
simply duplicating it.

Helped-by: Jeff King <p...@peff.net>
Signed-off-by: Stefan Beller <sbel...@google.com>
Signed-off-by: Marc Branchaud <marcn...@xiplink.com>

---

Tested the sed "2a" command's escaping in both Ubuntu 17.04 and FreeBSD 10.3.
Threw in a little indenting so that it isn't too ugly.

 diff.c   | 2 +-
 t/t4051-diff-function-context.sh | 3 ++-
 t/t4061-diff-indent.sh   | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index da96577ea..2c47ccb4a 100644
--- a/diff.c
+++ b/diff.c
@@ -27,7 +27,7 @@
 #endif
 
 static int diff_detect_rename_default;
-static int diff_indent_heuristic; /* experimental */
+static int diff_indent_heuristic = 1;
 static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
 static int diff_use_color_default = -1;
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 6154acb45..3e6b485ec 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -72,7 +72,8 @@ test_expect_success 'setup' '
 
# overlap function context of 1st change and -u context of 2nd change
grep -v "delete me from hello" <"$dir/hello.c" >file.c &&
-   sed 2p <"$dir/dummy.c" >>file.c &&
+   sed "2a\\
+extra line" <"$dir/dummy.c" >>file.c &&
commit_and_tag changed_hello_dummy file.c &&
 
git checkout initial &&
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 13d3dc96a..56d7d7760 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -153,7 +153,7 @@ test_expect_success 'prepare' '
 '
 
 test_expect_success 'diff: ugly spaces' '
-   git diff old new -- spaces.txt >out &&
+   git diff --no-indent-heuristic old new -- spaces.txt >out &&
compare_diff spaces-expect out
 '
 
@@ -183,7 +183,7 @@ test_expect_success 'diff: --indent-heuristic with 
--histogram' '
 '
 
 test_expect_success 'diff: ugly functions' '
-   git diff old new -- functions.c >out &&
+   git diff --no-indent-heuristic old new -- functions.c >out &&
compare_diff functions-expect out
 '
 
@@ -193,7 +193,7 @@ test_expect_success 'diff: nice functions with 
--indent-heuristic' '
 '
 
 test_expect_success 'blame: ugly spaces' '
-   git blame old..new -- spaces.txt >out-blame &&
+   git blame --no-indent-heuristic old..new -- spaces.txt >out-blame &&
compare_blame spaces-expect out-blame
 '
 
-- 
2.13.0.rc1.15.gf67d331ad

[PATCHv3 4/4] add--interactive: drop diff.indentHeuristic handling

[Marc Branchaud]

From: Jeff King <p...@peff.net>

Now that diff.indentHeuristic is handled automatically by the plumbing
commands, there's no need to propagate it manually.

Signed-off-by: Jeff King <p...@peff.net>
Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 git-add--interactive.perl | 4 
 1 file changed, 4 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 709a5f6ce..79d675b5b 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -46,7 +46,6 @@ my ($diff_new_color) =
 my $normal_color = $repo->get_color("", "reset");
 
 my $diff_algorithm = $repo->config('diff.algorithm');
-my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');
 my $diff_filter = $repo->config('interactive.difffilter');
 
 my $use_readkey = 0;
@@ -730,9 +729,6 @@ sub parse_diff {
if (defined $diff_algorithm) {
splice @diff_cmd, 1, 0, "--diff-algorithm=${diff_algorithm}";
}
-   if ($diff_indent_heuristic) {
-   splice @diff_cmd, 1, 0, "--indent-heuristic";
-   }
if (defined $patch_mode_revision) {
push @diff_cmd, get_diff_reference($patch_mode_revision);
}
-- 
2.13.0.rc1.15.gf67d331ad

[PATCHv3 0/4] Make diff plumbing commands respect the indentHeuristic.

[Marc Branchaud]

On 2017-04-29 09:14 AM, Jeff King wrote:
> On Sat, Apr 29, 2017 at 08:40:52AM -0400, Jeff King wrote:
> 
>> On Fri, Apr 28, 2017 at 06:33:12PM -0400, Marc Branchaud wrote:
>>
>>> v2: Fixed up the commit messages and added tests.
>>>
>>> Marc Branchaud (2):
>>>   diff: make the indent heuristic part of diff's basic configuration
>>>   diff: have the diff-* builtins configure diff before initializing
>>> revisions
>>>
>>> Stefan Beller (1):
>>>   diff: enable indent heuristic by default
>>
>> Thanks, these look fine to me. I'd like to get an ACK from Michael, in
>> case he had some other reason for omitting them from git_diff_ui_config
>> (from my recollection, it's probably just a mix of conservatism and
>> following what the compaction heuristic had done).
> 
> Sorry, I spoke too soon. The third one needs a few test adjustments
> squashed in to pass the tests.

Doh!  That'll teach me to try to do this stuff at the end of a Friday...

One more try, then:

Changes since v2:

  Patch 1/4 : Unchanged.

  Patch 2/4 : Mentioned how the new behaviour matches the diff Porcelain.

  Patch 3/4 : Updated the tests.

  Patch 4/4 : (New) Jeff's add--interactive patch.

Thanks for all the help, Jeff!

M.


Jeff King (1):
  add--interactive: drop diff.indentHeuristic handling

Marc Branchaud (2):
  diff: make the indent heuristic part of diff's basic configuration
  diff: have the diff-* builtins configure diff before initializing
revisions

Stefan Beller (1):
  diff: enable indent heuristic by default

 builtin/diff-files.c |  2 +-
 builtin/diff-index.c |  2 +-
 builtin/diff-tree.c  |  2 +-
 diff.c   |  8 ++---
 git-add--interactive.perl|  4 ---
 t/t4051-diff-function-context.sh |  3 +-
 t/t4061-diff-indent.sh   | 72 ++--
 7 files changed, 78 insertions(+), 15 deletions(-)

-- 
2.13.0.rc1.15.gf67d331ad

[PATCHv3 1/4] diff: make the indent heuristic part of diff's basic configuration

[Marc Branchaud]

This heuristic was originally introduced as an experimental feature,
and therefore part of the UI configuration.

But the user often sees diffs generated by plumbing commands like
diff-tree.  Moving the indent heuristic into diff's basic configuration
prepares the way for diff plumbing commands to respect the setting.

The heuristic itself merely makes the diffs more aesthetically
pleasing, without changing their correctness.  Scripts that rely on
the diff plumbing commands should not care whether or not the heuristic
is employed.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 diff.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 11eef1c85..da96577ea 100644
--- a/diff.c
+++ b/diff.c
@@ -290,9 +290,6 @@ int git_diff_ui_config(const char *var, const char *value, 
void *cb)
return 0;
}
 
-   if (git_diff_heuristic_config(var, value, cb) < 0)
-   return -1;
-
if (!strcmp(var, "diff.wserrorhighlight")) {
int val = parse_ws_error_highlight(value);
if (val < 0)
@@ -351,6 +348,9 @@ int git_diff_basic_config(const char *var, const char 
*value, void *cb)
if (starts_with(var, "submodule."))
return parse_submodule_config_option(var, value);
 
+   if (git_diff_heuristic_config(var, value, cb) < 0)
+   return -1;
+
return git_default_config(var, value, cb);
 }
 
-- 
2.13.0.rc1.15.gf67d331ad

[PATCHv3 2/4] diff: have the diff-* builtins configure diff before initializing revisions

[Marc Branchaud]

This matches how the diff Porcelain works.  It makes the plumbing commands
respect diff's configuration options, such as indentHeuristic, because
init_revisions() calls diff_setup() which fills in the diff_options struct.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 builtin/diff-files.c   |  2 +-
 builtin/diff-index.c   |  2 +-
 builtin/diff-tree.c|  2 +-
 t/t4061-diff-indent.sh | 66 ++
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 15c61fd8d..a572da9d5 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -20,9 +20,9 @@ int cmd_diff_files(int argc, const char **argv, const char 
*prefix)
int result;
unsigned options = 0;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
precompose_argv(argc, argv);
 
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 1af373d00..f084826a2 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -17,9 +17,9 @@ int cmd_diff_index(int argc, const char **argv, const char 
*prefix)
int i;
int result;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
precompose_argv(argc, argv);
 
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 326f88b65..36a3a1976 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -105,9 +105,9 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
struct setup_revision_opt s_r_opt;
int read_stdin = 0;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(opt, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
opt->abbrev = 0;
opt->diff = 1;
opt->disable_stdin = 1;
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 556450609..13d3dc96a 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -213,4 +213,70 @@ test_expect_success 'blame: --no-indent-heuristic 
overrides config' '
compare_blame spaces-expect out-blame2
 '
 
+test_expect_success 'diff-tree: nice spaces with --indent-heuristic' '
+   git diff-tree --indent-heuristic -p old new -- spaces.txt 
>out-diff-tree-compacted &&
+   compare_diff spaces-compacted-expect out-diff-tree-compacted
+'
+
+test_expect_success 'diff-tree: nice spaces with diff.indentHeuristic' '
+   git -c diff.indentHeuristic=true diff-tree -p old new -- spaces.txt 
>out-diff-tree-compacted2 &&
+   compare_diff spaces-compacted-expect out-diff-tree-compacted2
+'
+
+test_expect_success 'diff-tree: --no-indent-heuristic overrides config' '
+   git -c diff.indentHeuristic=true diff-tree --no-indent-heuristic -p old 
new -- spaces.txt >out-diff-tree &&
+   compare_diff spaces-expect out-diff-tree
+'
+
+test_expect_success 'diff-index: nice spaces with --indent-heuristic' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git diff-index --indent-heuristic -p old -- spaces.txt 
>out-diff-index-compacted &&
+   compare_diff spaces-compacted-expect out-diff-index-compacted &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-index: nice spaces with diff.indentHeuristic' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git -c diff.indentHeuristic=true diff-index -p old -- spaces.txt 
>out-diff-index-compacted2 &&
+   compare_diff spaces-compacted-expect out-diff-index-compacted2 &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-index: --no-indent-heuristic overrides config' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git -c diff.indentHeuristic=true diff-index --no-indent-heuristic -p 
old -- spaces.txt >out-diff-index &&
+   compare_diff spaces-expect out-diff-index &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-files: nice spaces with diff.indentHeuristic' '
+   git checkout -B diff-files &&
+   git reset HEAD~ &&
+   git -c diff.indentHeuristic=true diff-files -p spaces.txt 
>out-diff-files-raw &&
+   grep -v index out-diff-files-raw >out-diff-files-compacted &&
+   compare_diff spaces-compacted-expect out-diff-files-compacted &&
+   git checkout -f master
+'
+
+test_expect_

[PATCHv2 2/3] diff: have the diff-* builtins configure diff before initializing revisions

[Marc Branchaud]

This makes the commands respect diff configuration options, such as
indentHeuristic, because init_revisions() calls diff_setup() which fills
in the diff_options struct.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 builtin/diff-files.c   |  2 +-
 builtin/diff-index.c   |  2 +-
 builtin/diff-tree.c|  2 +-
 t/t4061-diff-indent.sh | 66 ++
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 15c61fd8d..a572da9d5 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -20,9 +20,9 @@ int cmd_diff_files(int argc, const char **argv, const char 
*prefix)
int result;
unsigned options = 0;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
precompose_argv(argc, argv);
 
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 1af373d00..f084826a2 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -17,9 +17,9 @@ int cmd_diff_index(int argc, const char **argv, const char 
*prefix)
int i;
int result;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
precompose_argv(argc, argv);
 
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 326f88b65..36a3a1976 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -105,9 +105,9 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
struct setup_revision_opt s_r_opt;
int read_stdin = 0;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(opt, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
opt->abbrev = 0;
opt->diff = 1;
opt->disable_stdin = 1;
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 556450609..13d3dc96a 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -213,4 +213,70 @@ test_expect_success 'blame: --no-indent-heuristic 
overrides config' '
compare_blame spaces-expect out-blame2
 '
 
+test_expect_success 'diff-tree: nice spaces with --indent-heuristic' '
+   git diff-tree --indent-heuristic -p old new -- spaces.txt 
>out-diff-tree-compacted &&
+   compare_diff spaces-compacted-expect out-diff-tree-compacted
+'
+
+test_expect_success 'diff-tree: nice spaces with diff.indentHeuristic' '
+   git -c diff.indentHeuristic=true diff-tree -p old new -- spaces.txt 
>out-diff-tree-compacted2 &&
+   compare_diff spaces-compacted-expect out-diff-tree-compacted2
+'
+
+test_expect_success 'diff-tree: --no-indent-heuristic overrides config' '
+   git -c diff.indentHeuristic=true diff-tree --no-indent-heuristic -p old 
new -- spaces.txt >out-diff-tree &&
+   compare_diff spaces-expect out-diff-tree
+'
+
+test_expect_success 'diff-index: nice spaces with --indent-heuristic' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git diff-index --indent-heuristic -p old -- spaces.txt 
>out-diff-index-compacted &&
+   compare_diff spaces-compacted-expect out-diff-index-compacted &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-index: nice spaces with diff.indentHeuristic' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git -c diff.indentHeuristic=true diff-index -p old -- spaces.txt 
>out-diff-index-compacted2 &&
+   compare_diff spaces-compacted-expect out-diff-index-compacted2 &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-index: --no-indent-heuristic overrides config' '
+   git checkout -B diff-index &&
+   git reset --soft HEAD~ &&
+   git -c diff.indentHeuristic=true diff-index --no-indent-heuristic -p 
old -- spaces.txt >out-diff-index &&
+   compare_diff spaces-expect out-diff-index &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-files: nice spaces with diff.indentHeuristic' '
+   git checkout -B diff-files &&
+   git reset HEAD~ &&
+   git -c diff.indentHeuristic=true diff-files -p spaces.txt 
>out-diff-files-raw &&
+   grep -v index out-diff-files-raw >out-diff-files-compacted &&
+   compare_diff spaces-compacted-expect out-diff-files-compacted &&
+   git checkout -f master
+'
+
+test_expect_success 'diff-files: nice spaces with diff.indentHe

[PATCH 3/3] diff: enable indent heuristic by default

[Marc Branchaud]

From: Stefan Beller <sbel...@google.com>

The feature was included in v2.11 (released 2016-11-29) and we got no
negative feedback. Quite the opposite, all feedback we got was positive.

Turn it on by default. Users who dislike the feature can turn it off
by setting diff.compactionHeuristic (which includes plumbing commands,
see prior patches).

Signed-off-by: Stefan Beller <sbel...@google.com>
Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index da96577ea..2c47ccb4a 100644
--- a/diff.c
+++ b/diff.c
@@ -27,7 +27,7 @@
 #endif
 
 static int diff_detect_rename_default;
-static int diff_indent_heuristic; /* experimental */
+static int diff_indent_heuristic = 1;
 static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
 static int diff_use_color_default = -1;
-- 
2.13.0.rc1.15.gf67d331ad

[PATCHv2 1/3] diff: make the indent heuristic part of diff's basic configuration

[Marc Branchaud]

This heuristic was originally introduced as an experimental feature,
and therefore part of the UI configuration.

But the user often sees diffs generated by plumbing commands like
diff-tree.  Moving the indent heuristic into diff's basic configuration
prepares the way for diff plumbing commands to respect the setting.

The heuristic itself merely makes the diffs more aesthetically
pleasing, without changing their correctness.  Scripts that rely on
the diff plumbing commands should not care whether or not the heuristic
is employed.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 diff.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 11eef1c85..da96577ea 100644
--- a/diff.c
+++ b/diff.c
@@ -290,9 +290,6 @@ int git_diff_ui_config(const char *var, const char *value, 
void *cb)
return 0;
}
 
-   if (git_diff_heuristic_config(var, value, cb) < 0)
-   return -1;
-
if (!strcmp(var, "diff.wserrorhighlight")) {
int val = parse_ws_error_highlight(value);
if (val < 0)
@@ -351,6 +348,9 @@ int git_diff_basic_config(const char *var, const char 
*value, void *cb)
if (starts_with(var, "submodule."))
return parse_submodule_config_option(var, value);
 
+   if (git_diff_heuristic_config(var, value, cb) < 0)
+   return -1;
+
return git_default_config(var, value, cb);
 }
 
-- 
2.13.0.rc1.15.gf67d331ad

[PATCHv2 0/3] Make diff plumbing commands respect the indentHeuristic.

[Marc Branchaud]

v2: Fixed up the commit messages and added tests.

Marc Branchaud (2):
  diff: make the indent heuristic part of diff's basic configuration
  diff: have the diff-* builtins configure diff before initializing
revisions

Stefan Beller (1):
  diff: enable indent heuristic by default

 builtin/diff-files.c   |  2 +-
 builtin/diff-index.c   |  2 +-
 builtin/diff-tree.c|  2 +-
 diff.c |  8 +++---
 t/t4061-diff-indent.sh | 66 ++
 5 files changed, 73 insertions(+), 7 deletions(-)

-- 
2.13.0.rc1.15.gf67d331ad

[PATCH 0/2] Make diff plumbing commands respect the indentHeuristic.

[Marc Branchaud]

So here's my attempt at fixing this.

The thing I was missing is that init_revisions() calls diff_setup(), which
sets the xdl options.  It's therefore necessary to have the
diff_indent_heuristic flag set before calling init_revisions().

A naive way to get the indentHeuristic config option respected in the
diff-* plumbing commands is to make them use the git_diff_heuristic_config()
callback right at the start of their main cmd functions.

But I did not like that for two reasons:

* It would make these commands invoke git_config() twice.

* It doesn't avoid the problem if/when someone creates a new diff-something
  plumbing command, and forgets to set the diff_indent_heuristic flag before
  calling init_revisions().

So instead I chose to make the indentHeuristic option part of diff's basic
configuration, and in each of the diff plumbing commands I moved the call to
git_config() before the call to init_revisions().

This still doesn't really future-proof things for possible new diff plumbing
commands, because someone could still invoke init_revisions() before setting
up diff's basic configuration.  But I don't see an obvious way of ensuring
that the diff_indent_heuristic flag is respected regardless of when
diff_setup() is invoked.

M.

Marc Branchaud (2):
  Make the indent heuristic part of diff's basic configuration.
  Have the diff-* builtins configure diff before initializing revisions.

 builtin/diff-files.c | 2 +-
 builtin/diff-index.c | 2 +-
 builtin/diff-tree.c  | 2 +-
 diff.c   | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

[PATCH 2/2] Have the diff-* builtins configure diff before initializing revisions.

[Marc Branchaud]

This makes the commands respect diff configuration options, such as
indentHeuristic.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 builtin/diff-files.c | 2 +-
 builtin/diff-index.c | 2 +-
 builtin/diff-tree.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 15c61fd8d..a572da9d5 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -20,9 +20,9 @@ int cmd_diff_files(int argc, const char **argv, const char 
*prefix)
int result;
unsigned options = 0;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
precompose_argv(argc, argv);
 
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 1af373d00..f084826a2 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -17,9 +17,9 @@ int cmd_diff_index(int argc, const char **argv, const char 
*prefix)
int i;
int result;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
precompose_argv(argc, argv);
 
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 326f88b65..36a3a1976 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -105,9 +105,9 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
struct setup_revision_opt s_r_opt;
int read_stdin = 0;
 
+   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
init_revisions(opt, prefix);
gitmodules_config();
-   git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
opt->abbrev = 0;
opt->diff = 1;
opt->disable_stdin = 1;
-- 
2.13.0.rc1.15.g7dbea34e1.dirty

[PATCH 1/2] Make the indent heuristic part of diff's basic configuration.

[Marc Branchaud]

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---
 diff.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 11eef1c85..da96577ea 100644
--- a/diff.c
+++ b/diff.c
@@ -290,9 +290,6 @@ int git_diff_ui_config(const char *var, const char *value, 
void *cb)
return 0;
}
 
-   if (git_diff_heuristic_config(var, value, cb) < 0)
-   return -1;
-
if (!strcmp(var, "diff.wserrorhighlight")) {
int val = parse_ws_error_highlight(value);
if (val < 0)
@@ -351,6 +348,9 @@ int git_diff_basic_config(const char *var, const char 
*value, void *cb)
if (starts_with(var, "submodule."))
return parse_submodule_config_option(var, value);
 
+   if (git_diff_heuristic_config(var, value, cb) < 0)
+   return -1;
+
return git_default_config(var, value, cb);
 }
 
-- 
2.13.0.rc1.15.g7dbea34e1.dirty

BUG: diff-{index,files,tree} (and git-gui) do not respect the diff.indentHeuristic config setting

[Marc Branchaud]

So I have

diff.indentHeuristic = true

and I noticed that git-gui was not using the heuristic.  This is because 
git-gui uses diff-index, and that does not respect the config setting, 
even though it supports the --indent-heuristic option.


And it looks like diff-files and diff-tree also have the same problem.

I tried a couple of quick-n-dirty things to fix it in diff-index, 
without success, and I've run out of git-hacking tame, so all I can do 
for now is throw out a bug report.


diff-index.c explicitly says "no 'diff' UI options" since 83ad63cfeb 
("diff: do not use configuration magic at the core-level", 2006-07-08), 
so maybe this needs to be fixed in git-gui (and maybe elsewhere), but to 
me it feels like the diff-foo commands should respect the setting.


(CC'ing Michael Haggerty, who added the heuristic.)

(This is git v2.12.2.)

M.

Re: [PATCH] Do not require Python for the git-remote-{bzr,hg} placeholder scripts

[Marc Branchaud]

On 2017-03-03 05:57 AM, Sebastian Schuberth wrote:

It does not make sense for these placeholder scripts to depend on Python
just because the real scripts do. At the example of Git for Windows, we
would not even be able to see those warnings as it does not ship with
Python. So just use plain shell scripts instead.


Just a niggle:  This change moves the warning message from stderr to stdout.

M.


Signed-off-by: Sebastian Schuberth 
---
 contrib/remote-helpers/git-remote-bzr | 16 +++-
 contrib/remote-helpers/git-remote-hg  | 16 +++-
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr 
b/contrib/remote-helpers/git-remote-bzr
index 712a137..ccc4aea 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -1,13 +1,11 @@
-#!/usr/bin/env python
+#!/bin/sh

-import sys
-
-sys.stderr.write('WARNING: git-remote-bzr is now maintained independently.\n')
-sys.stderr.write('WARNING: For more information visit 
https://github.com/felipec/git-remote-bzr\n')
-
-sys.stderr.write('''WARNING:
+cat <<'EOT'
+WARNING: git-remote-bzr is now maintained independently.
+WARNING: For more information visit https://github.com/felipec/git-remote-bzr
+WARNING:
 WARNING: You can pick a directory on your $PATH and download it, e.g.:
-WARNING:   $ wget -O $HOME/bin/git-remote-bzr \\
+WARNING:   $ wget -O $HOME/bin/git-remote-bzr \
 WARNING: 
https://raw.github.com/felipec/git-remote-bzr/master/git-remote-bzr
 WARNING:   $ chmod +x $HOME/bin/git-remote-bzr
-''')
+EOT
diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 4255ad6..dfda44f 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -1,13 +1,11 @@
-#!/usr/bin/env python
+#!/bin/sh

-import sys
-
-sys.stderr.write('WARNING: git-remote-hg is now maintained independently.\n')
-sys.stderr.write('WARNING: For more information visit 
https://github.com/felipec/git-remote-hg\n')
-
-sys.stderr.write('''WARNING:
+cat <<'EOT'
+WARNING: git-remote-hg is now maintained independently.
+WARNING: For more information visit https://github.com/felipec/git-remote-hg
+WARNING:
 WARNING: You can pick a directory on your $PATH and download it, e.g.:
-WARNING:   $ wget -O $HOME/bin/git-remote-hg \\
+WARNING:   $ wget -O $HOME/bin/git-remote-hg \
 WARNING: https://raw.github.com/felipec/git-remote-hg/master/git-remote-hg
 WARNING:   $ chmod +x $HOME/bin/git-remote-hg
-''')
+EOT

--
https://github.com/git/git/pull/333

Re: [RFC][Git GUI] Make Commit message field in git GUI re sizable.

[Marc Branchaud]

On 2017-02-22 06:59 AM, Jessie Hernandez wrote:

HI,

the reason why it is fixed, is because commit messages should be
wrapped at 76 characters to be used in mails. So it helps you with the
wrapping.

Bert


Right ok. I understand.
Knowing this I think I might start writing my commit messages differently
then.


You can configure gui.commitMsgWidth if you don't like the default 
(which is 75, not 76).


M.



Thank you for this.

Regards

-
Jessie Hernandez




On Wed, Feb 22, 2017 at 10:27 AM, Jessie Hernandez
 wrote:

Hi all,

I have been using git for a few years now and really like the software.
I have a small annoyance and was wondering if I could get the
communities
view on this.

When using git GUI I find it handy to be able to re-size the "Unstaged
Changes" and the "Staged Changed" fields.

I would like the same thing for the "Commit Message" field, or to have
it
re-size with the git GUI window.

I can re-size the "Commit Message" vertically when making the "Modified"
panel smaller.

Does this make sense?
I would be happy to get into more detail if that is necessary or if
something is not clear.

Thank you.

-
Jessie Hernandez

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

[Marc Branchaud]

On 2016-12-19 11:44 AM, Michael Haggerty wrote:

This patch series changes a bunch of details about how remote-tracking
references are rendered in the commit list of gitk:


I don't see this series in git v2.12.0-rc0, nor in Paul's gitk repo.

I hope this is an oversight, and not that the series got dropped for 
some reason.


M.




* Omit the "remote/" prefix on normal remote-tracking references. They
  are already distinguished via their two-tone rendering and (usually)
  longer names, and this change saves a lot of visual clutter and
  horizontal space.

* Render remote-tracking references that have more than the usual
  three slashes like

  origin/foo/bar
  ^^^

  rather than

  origin/foo/bar (formerly remotes/origin/foo/bar)
  ^^^  ^^^

  , where the indicated part is the prefix that is rendered in a
  different color. Usually, such a reference represents a remote
  branch that contains a slash in its name, so the new split more
  accurately portrays the separation between remote name and remote
  branch name.

* Introduce a separate constant to specify the background color used
  for the branch name part of remote-tracking references, to allow it
  to differ from the color used for local branches (which by default
  is bright green).

* Change the default background colors for remote-tracking branches to
  light brown and brown (formerly they were pale orange and bright
  green).

I understand that the colors of pixels on computer screens is an even
more emotional topic that that of bikesheds, so I implemented the last
change as a separate commit, the last one in the series. Feel free to
drop it if you don't want the default color change.

Along the way, I did a bunch of refactoring in the area to make these
changes easier, and introduced a constant for the background color of
"other" references so that it can also be adjusted by users.

(Unfortunately, these colors can only be adjusted by editing the
configuration file. Someday it would be nice to allow them to be
configured via the preferences dialog.)

It's been a while since I've written any Tcl code, so I apologize in
advance for any howlers :-)

This branch applies against the `master` branch in
git://ozlabs.org/~paulus/gitk.

Michael

Michael Haggerty (13):
  gitk: when processing tag labels, don't use `marks` as scratch space
  gitk: keep track of tag types in a separate `types` array
  gitk: use a type "tags" to indicate abbreviated tags
  gitk: use a type "mainhead" to indicate the main HEAD branch
  gitk: fill in `wvals` as the tags are first processed
  gitk: simplify regexp
  gitk: extract a method `remotereftext`
  gitk: only change the color of the "remote" part of remote refs
  gitk: shorten labels displayed for modern remote-tracking refs
  gitk: use type "remote" for remote-tracking references
  gitk: introduce a constant otherrefbgcolor
  gitk: add a configuration setting `remoterefbgcolor`
  gitk: change the default colors for remote-tracking references

 gitk | 114 ---
 1 file changed, 76 insertions(+), 38 deletions(-)

Re: [RFC] stash --continue

[Marc Branchaud]

On 2017-01-19 04:30 PM, Johannes Schindelin wrote:


At this point I will stop commenting on this issue, as I have said all
that I wanted to say about it, at least once. If I failed to get my points
across so far, I simply won't be understood.


Yes, we're obviously looking at this from completely different perspectives.

Stephan (or whoever) if you decide to do this work, I will be content 
with whichever way you choose to go.


M.

Re: [RFC] stash --continue

[Marc Branchaud]

On 2017-01-19 10:49 AM, Johannes Schindelin wrote:

Hi Marc,

On Wed, 18 Jan 2017, Marc Branchaud wrote:


On 2017-01-18 11:34 AM, Johannes Schindelin wrote:


On Wed, 18 Jan 2017, Marc Branchaud wrote:


On 2017-01-16 05:54 AM, Johannes Schindelin wrote:


On Mon, 16 Jan 2017, Stephan Beyer wrote:


a git-newbie-ish co-worker uses git-stash sometimes. Last time
he used "git stash pop", he got into a merge conflict. After he
resolved the conflict, he did not know what to do to get the
repository into the wanted state. In his case, it was only "git
add " followed by a "git reset" and a "git stash
drop", but there may be more involved cases when your index is
not clean before "git stash pop" and you want to have your index
as before.

This led to the idea to have something like "git stash
--continue"[1]


More like "git stash pop --continue". Without the "pop" command,
it does not make too much sense.


Why not?  git should be able to remember what stash command created
the conflict.  Why should I have to?  Maybe the fire alarm goes off
right when I run the stash command, and by the time I get back to it
I can't remember which operation I did.  It would be nice to be able
to tell git to "just finish off (or abort) the stash operation,
whatever it was".


That reeks of a big potential for confusion.

Imagine for example a total Git noob who calls `git stash list`,
scrolls two pages down, then hits `q` by mistake. How would you
explain to that user that `git stash --continue` does not continue
showing the list at the third page?


Sorry, but I have trouble taking that example seriously.  It assumes
such a level of "noobness" that the user doesn't even understand how
standard command output paging works, not just with git but with any
shell command.


Yeah, well, I thought you understood what I meant.

The example was the best I could come up with quickly, and it only tried
to show that there are *other* stash operations that one might perceive
to happen at the same time as the "pop" operation, so your flimsical
comment "why not continue the latest operation" may very well be
ambiguous.

And if it is not ambiguous in "stash", it certainly will be in other Git
operations. And therefore, having a DWIM in "stash" to allow "--continue"
without any specific subcommand, but not having it in other Git commands,
is just a very poor user interface design. It is prone to confuse users,
which is always a hallmark of a bad user interface.


Please don't underestimate the power of syntactic consistency in helping 
users achieve their goals.  Having some commands use "git foo 
--continue" while others use "git foo bar --continue" *will* confuse 
people, regardless of how logical the reasons for those differences.


But in the case of stash, I still don't see the utility in having 
operation-specific continuation.  Consider the following sequence (as 
you say, this doesn't work yet, but making it work seems reasonable):


git stash pop  # creates some conflicts
git stash apply stash@{4} # creates some other conflicts
# (User resolves the conflicts created by the pop.)
git stash pop --continue

Given the description of the original proposal (do "git reset; git stash 
drop"), what's the state of the index and the working tree?


In particular, what has the user gained by continuing just that pop?

Another thing to ask is, how common is such a scenario likely to be?  I 
suggest that it will be far more common for users to resolve all the 
conflicts and then want to continue all their interrupted stash 
operations.  If so, fussily forcing them to explicitly continue the pop 
and the apply is just a waste.



Hence my objection to "git stash --continue". No argument in favor of "git
stash --continue" I heard so far comes even close to being convincing.


Well, what about the potential for a slippery slope?  If the user is 
forced to be specific about continuing either a pop or an apply, why 
wouldn't git allow them to be specific about *which* pop or apply they 
want to continue?  Consider another hypothetical scenario:


git stash pop  # creates some conflicts
git stash pop  # creates some more conflicts
git stash pop  # creates even more conflicts
# (User resolves the conflicts created by second pop.)
git stash pop --continue
# Oops, there's still some unresovled pops!

Obviously the user isn't ready to finish off all the pops, so they'll 
want some way to specify which pop to continue.  Dealing with that just 
feels like a lot of work for minimal benefit.



Even worse: `git stash` (without arguments) defaults to the `save`
operation, so any user who does not read the documentation (and who
does?) would assume that `git stash --continue` *also* implie

Re: [RFC] stash --continue

[Marc Branchaud]

On 2017-01-18 11:34 AM, Johannes Schindelin wrote:

Hi Marc,

On Wed, 18 Jan 2017, Marc Branchaud wrote:


On 2017-01-16 05:54 AM, Johannes Schindelin wrote:


On Mon, 16 Jan 2017, Stephan Beyer wrote:


a git-newbie-ish co-worker uses git-stash sometimes. Last time he
used "git stash pop", he got into a merge conflict. After he
resolved the conflict, he did not know what to do to get the
repository into the wanted state. In his case, it was only "git add
" followed by a "git reset" and a "git stash drop",
but there may be more involved cases when your index is not clean
before "git stash pop" and you want to have your index as before.

This led to the idea to have something like "git stash
--continue"[1]


More like "git stash pop --continue". Without the "pop" command, it
does not make too much sense.


Why not?  git should be able to remember what stash command created the
conflict.  Why should I have to?  Maybe the fire alarm goes off right when I
run the stash command, and by the time I get back to it I can't remember
which operation I did.  It would be nice to be able to tell git to "just
finish off (or abort) the stash operation, whatever it was".


That reeks of a big potential for confusion.

Imagine for example a total Git noob who calls `git stash list`, scrolls
two pages down, then hits `q` by mistake. How would you explain to that
user that `git stash --continue` does not continue showing the list at the
third page?


Sorry, but I have trouble taking that example seriously.  It assumes 
such a level of "noobness" that the user doesn't even understand how 
standard command output paging works, not just with git but with any 
shell command.



Even worse: `git stash` (without arguments) defaults to the `save`
operation, so any user who does not read the documentation (and who does?)
would assume that `git stash --continue` *also* implies `save`.


Like the first example, your user is trying to "continue" a command that 
is already complete.  It's like try to do "git rebase --continue" when 
there's no rebase operation underway.


Now, maybe there is some way for "git stash save" (implied or explicit) 
to stop partway through the operation.  I can't imagine such a situation 
(out of disk space, maybe?), particularly where the user would expect 
"git stash save" to leave things in a half-finished state.  To me "git 
stash save" should be essentially all-or-nothing.


However, if there were such a partial-failure scenario, then I think it 
would be perfectly reasonable for "git stash --continue" to finish the 
save operation, assuming that the failure condition has been resolved.



If that was not enough, there would still be the overall design of Git's
user interface. You can call it confusing, inconsistent, with a lot of
room for improvement, and you would be correct. But none of Git's commands
has a `--continue` option that remembers the latest subcommand and
continues that. To introduce that behavior in `git stash` would disimprove
the situation.


I think it's more the case that none of the current continuable commands 
have subcommands (though I can't think of all the continuable or 
abortable operations offhand, so maybe I'm wrong).  I think we're 
discussing new UI ground here.


And since the pattern is already "git foo --continue", it seems more 
consistent to me for it to be "git stash --continue" as well. 
Especially since there can be only one partially-complete stash 
sub-operation at one time (per workdir, at least).  So there's no reason 
to change the pattern just for the stash command.


Think of it this way:  All the currently continuable/abortable commands 
put the repository in a shaky state, where performing certain other 
operations would be ill advised.  Attempting to start a rebase while a 
merge conflict is unresolved, for example.  IIRC, git actually tries to 
stop users from shooting their feet in this way.


And so it should be for the stash operation:  If applying a stash yields 
a conflict, it has to be resolved or aborted before something like a 
rebase or merge is attempted.  It doesn't matter which stash subcommand 
created the shaky situation.


In the long run, I think there's even the possibility of generic "git 
continue" and "git abort" commands, that simply continue or abort the 
current partially-complete operation, whatever it is.  (Isn't that the 
ultimate goal of all the "sequencer" work?  I admit I have not been 
following that effort.)



With every new feature, it is not enough to consider its benefits. You
always have to take the potential fallout into account, too.


Agreed.


At least `git stash pop --continue` would be consistent with all other
`--continue` options in Git that I can think of...


Alas, I disagree!

M.

Re: [RFC] stash --continue

[Marc Branchaud]

On 2017-01-16 05:54 AM, Johannes Schindelin wrote:

Hi Stephan,

On Mon, 16 Jan 2017, Stephan Beyer wrote:


a git-newbie-ish co-worker uses git-stash sometimes. Last time he used
"git stash pop", he got into a merge conflict. After he resolved the
conflict, he did not know what to do to get the repository into the
wanted state. In his case, it was only "git add "
followed by a "git reset" and a "git stash drop", but there may be more
involved cases when your index is not clean before "git stash pop" and
you want to have your index as before.

This led to the idea to have something like "git stash --continue"[1]


More like "git stash pop --continue". Without the "pop" command, it does
not make too much sense.


Why not?  git should be able to remember what stash command created the 
conflict.  Why should I have to?  Maybe the fire alarm goes off right 
when I run the stash command, and by the time I get back to it I can't 
remember which operation I did.  It would be nice to be able to tell git 
to "just finish off (or abort) the stash operation, whatever it was".


M.

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

[Marc Branchaud]

On 2016-12-22 03:15 AM, Michael Haggerty wrote:

On 12/21/2016 08:07 PM, Marc Branchaud wrote:

On 2016-12-20 07:05 PM, Michael Haggerty wrote:

On 12/20/2016 04:01 PM, Marc Branchaud wrote:

[...]
Please don't change the remotebgcolor default.

Also, perhaps the default remoterefbgcolor should be
set remoterefbgcolor $headbgcolor
?

I say this because when I applied the series, without the last patch, I
was miffed that the remote/ref colour had changed.


This is a one-time inconvenience that gitk developers will experience. I
doubt that users jump backwards and forwards in gitk versions very often.


In what way do you mean it's restricted to gitk developers?


Maybe I misunderstood your objection.

While developing this, I realized that the very first time your run gitk
(i.e., when you don't already have a configuration file), it writes the
then-current default colors into your configuration file. If you later
switch gitk versions to a version with different default colors, the
colors from the first-run version are preserved (unless the names of the
variables used to hold the colors are changed).

So if you would run the tip version of my branch first, then the parent
of that version, you would continue to see the colors as modified by the
final commit. If you then switch to the master version, the remote
branch names go back to green (because it goes back to using
`headbgcolor` again), but the remote prefix stays light brown. I thought
you were unhappy about some form of this unexpected persistence problem.
But this problem will mostly be experienced by gitk developers who jump
back and forth between versions.

A normal user probably mostly moves forward through version numbers, and
only occasionally. Such a user, jumping from master to the tip of my
branch (assuming they haven't customized anything), would see

* local branches stay lime
* remote branch prefixes stay pale orange (they don't change to light
brown because the pale orange color from master is stored in their
configuration file)
* remote branch names change from lime to brown (because the
`remoterefbgcolor` configuration setting didn't exist before)


Patch 12 introduces remoterefbgcolor, with a specific default value.
Prior to that, the "ref part" of remote refs was rendered with
headbgcolor.  Users who changed their headbgcolor are used to seeing the
"ref part" of remote refs in the same color as their local heads.
Applying patch 12 changes the "ref part" color of remote refs, for such
users.

All I'm saying is that make the remoterefbgcolor default be $headbgcolor
avoids this.


For somebody who thinks that most people will want local and
remote-tracking branch names to be rendered in the same color, your
suggestion would be an improvement.

But for somebody like me who thinks it is a good idea to render
remote-tracking branch names in a different color than local branch
names, this is a feature, not a bug. Even a user who explicitly
configured `headbgcolor` to, say, purple, wasn't necessarily expressing
a wish to have remote-tracking branch names rendered in purple. Until
now they had no choice to set these colors separately!

So even for somebody who configured this setting before, I think that
having the remote-tracking branch names change color when the user
upgrades to this version is a good thing, because it lets the user know
that these two things can now be colored independently. If they don't
like the new default brown color, such a user knows where to change it
(even to make it agree with `headbgcolor` if they want that).

But I understand that this is a matter of personal preference. I have
but one "vote" :-)


I think we understand each other, and that we disagree.  I don't feel 
strongly about it though, so if you want to go this way that's fine by me.


Your case would be helped if the various ref colours were exposed in the 
preferences dialog.  Someone who upgrades to your gitk and doesn't like 
the default remoterefbgcolor has to track down the right setting, close 
all running gitk instances, and hand-edit ~/.gitk.


But I think improving gitk's colour-preferences dialog is a bit beyond 
the scope of your topic...


M.

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

[Marc Branchaud]

On 2016-12-20 07:05 PM, Michael Haggerty wrote:

On 12/20/2016 04:01 PM, Marc Branchaud wrote:

On 2016-12-19 11:44 AM, Michael Haggerty wrote:

This patch series changes a bunch of details about how remote-tracking
references are rendered in the commit list of gitk:


Thanks for this!  I like the new, compact look very much!

That said, I remember when I was a new git user and I leaned heavily on
gitk to understand how references worked.  It was particularly
illuminating to see the remote references distinctly labeled, and the
fact that they were "remotes/origin/foo" gave me an Aha! moment where I
came to understand that the refs hierarchy is more flexible than just
the conventions coded into git itself.  I eventually felt free to create
my own, private ref hierarchies.

I am in no way opposed to this series.  I just wanted to point out that
there was some utility in those labels.  It makes me think that it might
be worthwhile for gitk to have a "raw-refs" mode, that shows the full
"refs/foo/bar/baz" paths of all the heads, tags, and whatever else.  It
could be a useful teaching tool for git.


Yes, I understand that the longer names might be useful for beginners,
and the full names even more so. However, I think once a user has that
"aha!" moment, the space wasted on all the redundant words is a real
impediment to gitk's usability. It is common to have a few references on
a single commit (especially if you pull from multiple remotes), in which
case the summary line is completely invisible (and therefore its context
menu is unreachable). I don't like the idea of dumbing down the UI
permanently based on what users need at the very beginning of their Git
usage.


I agree.


Would it be possible to use the short names in the UI but to add the
full names to a tooltip or to the context menu?


I don't know -- my Tk-fu is weak.


* Omit the "remote/" prefix on normal remote-tracking references. They


If you re-roll, s:remote/:remotes/:.


Thanks.


  are already distinguished via their two-tone rendering and (usually)
  longer names, and this change saves a lot of visual clutter and
  horizontal space.

* Render remote-tracking references that have more than the usual
  three slashes like

  origin/foo/bar
  ^^^

  rather than

  origin/foo/bar (formerly remotes/origin/foo/bar)
  ^^^  ^^^

  , where the indicated part is the prefix that is rendered in a
  different color. Usually, such a reference represents a remote
  branch that contains a slash in its name, so the new split more
  accurately portrays the separation between remote name and remote
  branch name.


*Love* this change!  :)


* Introduce a separate constant to specify the background color used
  for the branch name part of remote-tracking references, to allow it
  to differ from the color used for local branches (which by default
  is bright green).

* Change the default background colors for remote-tracking branches to
  light brown and brown (formerly they were pale orange and bright
  green).


Please don't change the remotebgcolor default.

Also, perhaps the default remoterefbgcolor should be
set remoterefbgcolor $headbgcolor
?

I say this because when I applied the series, without the last patch, I
was miffed that the remote/ref colour had changed.


This is a one-time inconvenience that gitk developers will experience. I
doubt that users jump backwards and forwards in gitk versions very often.


In what way do you mean it's restricted to gitk developers?

Patch 12 introduces remoterefbgcolor, with a specific default value. 
Prior to that, the "ref part" of remote refs was rendered with 
headbgcolor.  Users who changed their headbgcolor are used to seeing the 
"ref part" of remote refs in the same color as their local heads. 
Applying patch 12 changes the "ref part" color of remote refs, for such 
users.


All I'm saying is that make the remoterefbgcolor default be $headbgcolor 
avoids this.


But, honestly, I don't feel strongly about it.

M.

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

[Marc Branchaud]

On 2016-12-20 05:17 PM, Paul Mackerras wrote:

On Tue, Dec 20, 2016 at 10:01:15AM -0500, Marc Branchaud wrote:

On 2016-12-19 11:44 AM, Michael Haggerty wrote:

This patch series changes a bunch of details about how remote-tracking
references are rendered in the commit list of gitk:


Thanks for this!  I like the new, compact look very much!

That said, I remember when I was a new git user and I leaned heavily on gitk
to understand how references worked.  It was particularly illuminating to
see the remote references distinctly labeled, and the fact that they were
"remotes/origin/foo" gave me an Aha! moment where I came to understand that
the refs hierarchy is more flexible than just the conventions coded into git
itself.  I eventually felt free to create my own, private ref hierarchies.

I am in no way opposed to this series.  I just wanted to point out that
there was some utility in those labels.  It makes me think that it might be
worthwhile for gitk to have a "raw-refs" mode, that shows the full
"refs/foo/bar/baz" paths of all the heads, tags, and whatever else.  It
could be a useful teaching tool for git.


Do you think we should have a checkbox in the preferences dialog to
select whether to display the long form or the short form?


Mmmm, more knobs!

No, I don't think that's necessary.  Such a setting would probably just 
confuse people.  Plus it's not something anyone is likely to want to 
change anyway.  Maybe if there were an "Advanced" tab in the settings 
dialog, but even then it seems like overkill.


I suspect there are better ways to teach people about the refs hierarchy 
than cluttering up gitk.  These may even already exist -- I've been a 
git user for 8 years now, so I'm sure my perspective of the learning 
curve is skewed.


M.

Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references

[Marc Branchaud]

On 2016-12-19 11:44 AM, Michael Haggerty wrote:

This patch series changes a bunch of details about how remote-tracking
references are rendered in the commit list of gitk:


Thanks for this!  I like the new, compact look very much!

That said, I remember when I was a new git user and I leaned heavily on 
gitk to understand how references worked.  It was particularly 
illuminating to see the remote references distinctly labeled, and the 
fact that they were "remotes/origin/foo" gave me an Aha! moment where I 
came to understand that the refs hierarchy is more flexible than just 
the conventions coded into git itself.  I eventually felt free to create 
my own, private ref hierarchies.


I am in no way opposed to this series.  I just wanted to point out that 
there was some utility in those labels.  It makes me think that it might 
be worthwhile for gitk to have a "raw-refs" mode, that shows the full 
"refs/foo/bar/baz" paths of all the heads, tags, and whatever else.  It 
could be a useful teaching tool for git.



* Omit the "remote/" prefix on normal remote-tracking references. They


If you re-roll, s:remote/:remotes/:.


  are already distinguished via their two-tone rendering and (usually)
  longer names, and this change saves a lot of visual clutter and
  horizontal space.

* Render remote-tracking references that have more than the usual
  three slashes like

  origin/foo/bar
  ^^^

  rather than

  origin/foo/bar (formerly remotes/origin/foo/bar)
  ^^^  ^^^

  , where the indicated part is the prefix that is rendered in a
  different color. Usually, such a reference represents a remote
  branch that contains a slash in its name, so the new split more
  accurately portrays the separation between remote name and remote
  branch name.


*Love* this change!  :)


* Introduce a separate constant to specify the background color used
  for the branch name part of remote-tracking references, to allow it
  to differ from the color used for local branches (which by default
  is bright green).

* Change the default background colors for remote-tracking branches to
  light brown and brown (formerly they were pale orange and bright
  green).


Please don't change the remotebgcolor default.

Also, perhaps the default remoterefbgcolor should be
set remoterefbgcolor $headbgcolor
?

I say this because when I applied the series, without the last patch, I 
was miffed that the remote/ref colour had changed.


Plus I think there's utility in having local and remote branch names in 
the same colour, again especially for new users.  It helps emphasize 
their similarities, and demystify some of git's internal magic.


M.



I understand that the colors of pixels on computer screens is an even
more emotional topic that that of bikesheds, so I implemented the last
change as a separate commit, the last one in the series. Feel free to
drop it if you don't want the default color change.

Along the way, I did a bunch of refactoring in the area to make these
changes easier, and introduced a constant for the background color of
"other" references so that it can also be adjusted by users.

(Unfortunately, these colors can only be adjusted by editing the
configuration file. Someday it would be nice to allow them to be
configured via the preferences dialog.)

It's been a while since I've written any Tcl code, so I apologize in
advance for any howlers :-)

This branch applies against the `master` branch in
git://ozlabs.org/~paulus/gitk.

Michael

Michael Haggerty (13):
  gitk: when processing tag labels, don't use `marks` as scratch space
  gitk: keep track of tag types in a separate `types` array
  gitk: use a type "tags" to indicate abbreviated tags
  gitk: use a type "mainhead" to indicate the main HEAD branch
  gitk: fill in `wvals` as the tags are first processed
  gitk: simplify regexp
  gitk: extract a method `remotereftext`
  gitk: only change the color of the "remote" part of remote refs
  gitk: shorten labels displayed for modern remote-tracking refs
  gitk: use type "remote" for remote-tracking references
  gitk: introduce a constant otherrefbgcolor
  gitk: add a configuration setting `remoterefbgcolor`
  gitk: change the default colors for remote-tracking references

 gitk | 114 ---
 1 file changed, 76 insertions(+), 38 deletions(-)

[PATCHv2] Tweak help auto-correct phrasing.

[Marc Branchaud]

When auto-correct is enabled, an invalid git command prints a warning and
a continuation message, which differs depending on whether or not
help.autoCorrect is positive or negative.

With help.autoCorrect = 15:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing under the assumption that you meant 'log'
   in 1.5 seconds automatically...

With help.autoCorrect < 0:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing under the assumption that you meant 'log'

The continuation message's phrasing is awkward.  This commit cleans it up.
As a bonus, we now use full-sentence strings which make translation easier.

With help.autoCorrect = 15:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing in 1.5 seconds, assuming that you meant 'log'.

With help.autoCorrect < 0:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing under the assumption that you meant 'log'.

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---

Writing the commit message was more work than the commit!  :)

M.

 help.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/help.c b/help.c
index 53e2a67e00..fc56aa2d76 100644
--- a/help.c
+++ b/help.c
@@ -381,12 +381,18 @@ const char *help_unknown_cmd(const char *cmd)
clean_cmdnames(_cmds);
fprintf_ln(stderr,
   _("WARNING: You called a Git command named '%s', "
-"which does not exist.\n"
-"Continuing under the assumption that you meant 
'%s'"),
-   cmd, assumed);
-   if (autocorrect > 0) {
-   fprintf_ln(stderr, _("in %0.1f seconds 
automatically..."),
-   (float)autocorrect/10.0);
+"which does not exist."),
+  cmd);
+   if (autocorrect < 0)
+   fprintf_ln(stderr,
+  _("Continuing under the assumption that "
+"you meant '%s'."),
+  assumed);
+   else {
+   fprintf_ln(stderr,
+  _("Continuing in %0.1f seconds, "
+"assuming that you meant '%s'."),
+  (float)autocorrect/10.0, assumed);
sleep_millisec(autocorrect * 100);
}
return assumed;
-- 
2.11.0.1.g75fa99b

[PATCH] Tweak help auto-correct phrasing.

[Marc Branchaud]

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---

On 2016-12-18 07:48 PM, Chris Packham wrote:
>
> This feature already exists (although it's not interactive). See
> help.autoCorrect in the git-config man page. "git config
> help.autoCorrect -1" should to the trick.

Awesome, I was unaware of this feature.  Thanks!

I found the message it prints a bit awkward, so here's a patch to fix it up.

Instead of:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing under the assumption that you meant 'log'
   in 1.5 seconds automatically...

it's now:

   WARNING: You called a Git command named 'lgo', which does not exist.
   Continuing in 1.5 seconds under the assumption that you meant 'log'.

M.

 help.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/help.c b/help.c
index 53e2a67e00..55350c0673 100644
--- a/help.c
+++ b/help.c
@@ -381,12 +381,18 @@ const char *help_unknown_cmd(const char *cmd)
clean_cmdnames(_cmds);
fprintf_ln(stderr,
   _("WARNING: You called a Git command named '%s', "
-"which does not exist.\n"
-"Continuing under the assumption that you meant 
'%s'"),
-   cmd, assumed);
-   if (autocorrect > 0) {
-   fprintf_ln(stderr, _("in %0.1f seconds 
automatically..."),
-   (float)autocorrect/10.0);
+"which does not exist."),
+  cmd);
+   if (autocorrect < 0)
+   fprintf_ln(stderr,
+  _("Continuing under the assumption that "
+"you meant '%s'."),
+  assumed);
+   else {
+   fprintf_ln(stderr,
+  _("Continuing in %0.1f seconds under the "
+"assumption that you meant '%s'."),
+  (float)autocorrect/10.0, assumed);
sleep_millisec(autocorrect * 100);
}
return assumed;
-- 
2.11.0.dirty

[PATCH] Release note spelling and phrasing fixups.

[Marc Branchaud]

Signed-off-by: Marc Branchaud <marcn...@xiplink.com>
---

Mostly just missing words and what I feel are clarifications.

The biggest change is to the "git add -N" item.  Not 100% sure
I got it right.

M.

 Documentation/RelNotes/2.11.0.txt | 145 +++---
 1 file changed, 72 insertions(+), 73 deletions(-)

diff --git a/Documentation/RelNotes/2.11.0.txt 
b/Documentation/RelNotes/2.11.0.txt
index cea2a50..de5892e 100644
--- a/Documentation/RelNotes/2.11.0.txt
+++ b/Documentation/RelNotes/2.11.0.txt
@@ -57,39 +57,40 @@ UI, Workflows & Features
 
  * Even though "git hash-objects", which is a tool to take an
on-filesystem data stream and put it into the Git object store,
-   allowed to perform the "outside-world-to-Git" conversions (e.g.
+   can perform "outside-world-to-Git" conversions (e.g.
end-of-line conversions and application of the clean-filter), and
-   it had the feature on by default from very early days, its reverse
+   it has had this feature on by default from very early days, its reverse
operation "git cat-file", which takes an object from the Git object
-   store and externalize for the consumption by the outside world,
+   store and externalizes it for consumption by the outside world,
lacked an equivalent mechanism to run the "Git-to-outside-world"
conversion.  The command learned the "--filters" option to do so.
 
- * Output from "git diff" can be made easier to read by selecting
+ * Output from "git diff" can be made easier to read by intelligently selecting
which lines are common and which lines are added/deleted
-   intelligently when the lines before and after the changed section
-   are the same.  A command line option is added to help with the
-   experiment to find a good heuristics.
+   when the lines before and after the changed section
+   are the same.  A command line option (--indent-heuristic) and a
+   configuration variable (diff.indentHeuristic) are added to help with the
+   experiment to find good heuristics.
 
  * In some projects, it is common to use "[RFC PATCH]" as the subject
prefix for a patch meant for discussion rather than application.  A
-   new option "--rfc" is a short-hand for "--subject-prefix=RFC PATCH"
+   new format-patch option "--rfc" is a short-hand for "--subject-prefix=RFC 
PATCH"
to help the participants of such projects.
 
- * "git add --chmod=+x " added recently only toggled the
+ * "git add --chmod={+,-}x " only changed the
executable bit for paths that are either new or modified. This has
-   been corrected to flip the executable bit for all paths that match
+   been corrected to change the executable bit for all paths that match
the given pathspec.
 
  * When "git format-patch --stdout" output is placed as an in-body
-   header and it uses the RFC2822 header folding, "git am" failed to
+   header and it uses RFC2822 header folding, "git am" fails to
put the header line back into a single logical line.  The
underlying "git mailinfo" was taught to handle this properly.
 
  * "gitweb" can spawn "highlight" to show blob contents with
(programming) language-specific syntax highlighting, but only
when the language is known.  "highlight" can however be told
-   to make the guess itself by giving it "--force" option, which
+   to guess the language itself by giving it "--force" option, which
has been enabled.
 
  * "git gui" l10n to Portuguese.
@@ -109,19 +110,19 @@ UI, Workflows & Features
history leading to nth parent was looking the other way.
 
  * In recent versions of cURL, GSSAPI credential delegation is
-   disabled by default due to CVE-2011-2192; introduce a configuration
-   to selectively allow enabling this.
+   disabled by default due to CVE-2011-2192; introduce a http.delegation
+   configuration variable to selectively allow enabling this.
(merge 26a7b23429 ps/http-gssapi-cred-delegation later to maint).
 
  * "git mergetool" learned to honor "-O" to control the
order of paths to present to the end user.
 
  * "git diff/log --ws-error-highlight=" lacked the corresponding
-   configuration variable to set it by default.
+   configuration variable (diff.wsErrorHighlight) to set it by default.
 
- * "git ls-files" learned "--recurse-submodules" option that can be
-   used to get a listing of tracked files across submodules (i.e. this
-   only works with "--cached" option, not for listing untracked or
+ * "git ls-files" learned the "--recurse-submodules" option
+   to get a listing of tracked files across submodules (i.e. this
+   only works with the "--cached" option

Re: [ANNOUNCE] Git v2.11.0-rc3

[Marc Branchaud]

On 2016-11-23 06:21 PM, Junio C Hamano wrote:


 * The original command line syntax for "git merge", which was "git
   merge  HEAD ...", has been deprecated for quite some
   time, and "git gui" was the last in-tree user of the syntax.  This
   is finally fixed, so that we can move forward with the deprecation.


Is this still true, given j6t's recent patch at

http://public-inbox.org/git/e61cc267-a59b-3be1-29db-c49d56f52...@kdbg.org/T/

?

M.

Re: BUG: indent-with-non-tab always on

[Marc Branchaud]

On 2016-08-15 12:55 PM, Marc Branchaud wrote:

On 2016-08-15 11:00 AM, Philip Oakley wrote:

From: "Marc Branchaud" <marcn...@xiplink.com>

On 2016-08-12 07:45 PM, Philip Oakley wrote:

These are the quick fixes to Marc's comments to patches 5,6,11,
and a consequential change to 12.

Only the changed patches are included.


Looks good to me -- no further comments!

However, I still don't understand why git says 11/12 has "indent with
spaces" problems.


I'm guessing it's that the text is monospaced rather than tabbed, and
it's flagging that.

I'd noticed that it was highlighted in the Git gui (which I use to
visualise both the diff, the message and the part after the three dashes
at the same time).


Actually, it looks like an indent-with-non-tab problem, which is
supposed to be off by default.

Git doesn't complain about the patch if I set
core.whitespace = tabwidth=11
presumably because the patch uses 10 spaces to indent the offending lines.

But explicitly disabling that check with
core.whitespace = -indent-with-non-tab
doesn't work.

Unfortunately, I don't have time today to track this down.


Gah, never mind -- didn't realize it was turned on in 
Documentation/.gitattributes.


M.

--
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

BUG: indent-with-non-tab always on (was: Re: [PATCH v6 00/12] Update git revisions)

[Marc Branchaud]

On 2016-08-15 11:00 AM, Philip Oakley wrote:

From: "Marc Branchaud" <marcn...@xiplink.com>

On 2016-08-12 07:45 PM, Philip Oakley wrote:

These are the quick fixes to Marc's comments to patches 5,6,11,
and a consequential change to 12.

Only the changed patches are included.


Looks good to me -- no further comments!

However, I still don't understand why git says 11/12 has "indent with
spaces" problems.


I'm guessing it's that the text is monospaced rather than tabbed, and
it's flagging that.

I'd noticed that it was highlighted in the Git gui (which I use to
visualise both the diff, the message and the part after the three dashes
at the same time).


Actually, it looks like an indent-with-non-tab problem, which is 
supposed to be off by default.


Git doesn't complain about the patch if I set
core.whitespace = tabwidth=11
presumably because the patch uses 10 spaces to indent the offending lines.

But explicitly disabling that check with
core.whitespace = -indent-with-non-tab
doesn't work.

Unfortunately, I don't have time today to track this down.

M.

--
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 v6 00/12] Update git revisions

[Marc Branchaud]

On 2016-08-12 07:45 PM, Philip Oakley wrote:

These are the quick fixes to Marc's comments to patches 5,6,11,
and a consequential change to 12.

Only the changed patches are included.


Looks good to me -- no further comments!

However, I still don't understand why git says 11/12 has "indent with 
spaces" problems.


M.



Philip Oakley (12):
  doc: use 'symmetric difference' consistently
  doc: revisions - name the left and right sides
  doc: show the actual left, right, and boundary marks
  doc: revisions: give headings for the two and three dot notations
  doc: revisions: extra clarification of ^! notation effects
  doc: revisions: single vs multi-parent notation comparison
  doc: gitrevisions - use 'reachable' in page description
  doc: gitrevisions - clarify 'latter case' is revision walk
  doc: revisions  - define `reachable`
  doc: revisions - clarify reachability examples
  doc: revisions: show revision expansion in examples
  doc: revisions: sort examples and fix alignment of the unchanged

 Documentation/gitk.txt |   2 +-
 Documentation/gitrevisions.txt |   6 +-
 Documentation/pretty-formats.txt   |   2 +-
 Documentation/rev-list-options.txt |   4 +-
 Documentation/revisions.txt| 125 -
 5 files changed, 88 insertions(+), 51 deletions(-)


--
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 v5 00/12] Update git revisions

[Marc Branchaud]

On 2016-08-12 03:07 AM, Philip Oakley wrote:

[2nd attempt : ISP troubles]

This has grown like topsy from a little two patch series that tried to
name the 2-dots notation [1] into this extended set of tweaks.


Honestly, I start just trying point out something concrete, like 
misrendered headers, but then I figure since I'm sending a review 
comment anyway I might as well go in-depth on the rest of the patch.


I think I'm sticking to substantive comments, but if I'm getting too 
picky please just swat me down!


M.

--
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 v5 11/12] doc: revisions: show revision expansion in examples

[Marc Branchaud]

On 2016-08-12 03:07 AM, Philip Oakley wrote:

The revisions examples show the revison arguments and the selected
commits, but do not show the intermediate step of the expansion of
the special 'range' notations. Extend the examples, including an
all-parents multi-parent merge commit example.

Sort the examples and fix the alignment for those unaffected
in the next commit.

Signed-off-by: Philip Oakley 
---
new
Cc: Jakub Narębski 
---
 Documentation/revisions.txt | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 70864d5..ac7dd8e 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -326,16 +326,23 @@ Revision Range Summary
   as giving commit '' and then all its parents prefixed with
   '{caret}' to exclude them (and their ancestors).

-Here are a handful of examples:
+Here are a handful of examples using the Loeliger illustration above:

+   Args   Expansion   Selection


I think "Result" would be better than "Selection" here.

Also, shouldn't all the ^ in these examples be {caret}?  (I likely just 
don't understand the rationale for using {caret} in some places and ^ in 
others...)



DG H D
D F  G H I J D F
^G D H D
^D B E I J F B
-   B..C C
-   B...CG H D E B C
+   B..C   = ^B C  C
+   B...C  = B ^F CG H D E B C
^D B C   E I J F B C
CI J F C
-   C^@  I J F
-   C^!  C
-   F^! DG H D F
+   C^@= C^1


I have a mixed reaction to showing this "C^1" expansion, and the "B^1 
B^2 B^3" one as well.  I see the appeal of showing the parent notation, 
but really that was already explained to death in the first section. 
Here it's distracting.  I think it's clearer for the reader to remove 
these expansions and just use the node names from the illustration.



+  = F I J F
+   B^@= B^1 B^2 B^3
+  = D E F D G H E F I J
+   C^!= C ^C^1


I think this expansion might be better expressed as "C ^C^@".  It'll be 
the same for "B^! = B ^B^@" as well, which demonstrates a nice 
consistency and also helps to emphasize the meaning of the ^@ notation.



+  = C ^F  C
+   B^! = B ^B^1 ^B^2 ^B^3
+   = B ^D ^E ^F   B


The layout of these last two lines doesn't match the others.  They 
should be:


   B^!= B ^B^1 ^B^2   ^B^3
  = B ^D ^E ^FB

I see that the next patch fixes the layout of the unchanged examples, 
but it leaves these two unaligned.


M.

--
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 v5 06/12] doc: revisions: single vs multi-parent notation comparison

[Marc Branchaud]

On 2016-08-12 03:07 AM, Philip Oakley wrote:

Signed-off-by: Philip Oakley 
---
new
Junio's final comment 
https://public-inbox.org/git/xmqqwpkq6b4d.fsf%40gitster.mtv.corp.google.com/
---
 Documentation/revisions.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 0b5044d..934d071 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -284,6 +284,10 @@ The 'r1{caret}@' notation means all parents of 'r1'.
 'r1{caret}!' notation includes commit 'r1' but excludes all of its parents.
 This is the single commit 'r1', if standalone.

+While '{caret}' was about specifying a single commit parent, these
+two notations consider all its parents. For example you can say
+'HEAD{caret}2^@', however you cannot say 'HEAD{caret}@{caret}2'.


That ^ should be {caret}, right?

M.

--
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 v5 05/12] doc: revisions: extra clarification of ^! notation effects

[Marc Branchaud]

On 2016-08-12 03:07 AM, Philip Oakley wrote:

Signed-off-by: Philip Oakley 
---
new
Cc: Jakub Narębski 
https://public-inbox.org/git/578E4F4A.2020708%40gmail.com/
---
 Documentation/revisions.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 3da0083..0b5044d 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -281,7 +281,8 @@ for naming a set that is formed by a commit and its parent 
commits.

 The 'r1{caret}@' notation means all parents of 'r1'.

-'r1{caret}!' includes commit 'r1' but excludes all of its parents.
+'r1{caret}!' notation includes commit 'r1' but excludes all of its parents.


This sentence should start with "The".


+This is the single commit 'r1', if standalone.


That reads awkwardly to me.  Perhaps

By itself, this notation denotes the single commit 'r1'.

?

M.

--
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 v5 04/12] doc: revisions: give headings for the two and three dot notations

[Marc Branchaud]

On 2016-08-12 03:07 AM, Philip Oakley wrote:

While there, also break out the other shorthand notations and
add a title for the revision range summary (which also appears
in git-rev-parse, so keep it mixed case).

We do not quote the notation within the headings as the asciidoc ->
docbook -> groff man viewer toolchain, particularly the docbook-groff
step, does not cope with two font changes, failing to return the heading
font to bold after the quotation of the notation.


Looks good --thanks!

M.

--
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 v4 4/8] doc: give headings for the two and three dot notations

[Marc Branchaud]

On 2016-07-21 03:54 PM, Philip Oakley wrote:

From: "Marc Branchaud" <marcn...@xiplink.com>

On 2016-07-20 05:10 PM, Philip Oakley wrote:

While there, also break out the other shorthand notations and
add a title for the revision range summary (which also appears
in git-rev-parse, so keep it mixed case).

Signed-off-by: Philip Oakley <philipoak...@iee.org>
---
  Documentation/revisions.txt | 58
-
  1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 6e9cd41..5b37283 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -242,35 +242,49 @@ specifying a single revision with the notation
described in the
  previous section means the set of commits reachable from that
  commit, following the commit ancestry chain.

-To exclude commits reachable from a commit, a prefix '{caret}'
-notation is used.  E.g. '{caret}r1 r2' means commits reachable
-from 'r2' but exclude the ones reachable from 'r1'.
-
-This set operation appears so often that there is a shorthand
-for it.  When you have two commits 'r1' and 'r2' (named according
-to the syntax explained in SPECIFYING REVISIONS above), you can ask
-for commits that are reachable from r2 excluding those that are
reachable
-from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
-
-A similar notation 'r1\...r2' is called symmetric difference
-of 'r1' and 'r2' and is defined as
-'r1 r2 --not $(git merge-base --all r1 r2)'.
-It is the set of commits that are reachable from either one of
-'r1' (left side) or 'r2' (right side) but not from both.
-
-In these two shorthands, you can omit one end and let it default to
HEAD.
+Commit Exclusions
+~
+
+'{caret}' (caret) Notation::
+ To exclude commits reachable from a commit, a prefix '{caret}'
+ notation is used.  E.g. '{caret}r1 r2' means commits reachable
+ from 'r2' but exclude the ones reachable from 'r1'.
+
+Dotted Range Notations
+~~
+
+The '..' (two-dot) Range Notation::
+ The '{caret}r1 r2' set operation appears so often that there is a
shorthand
+ for it.  When you have two commits 'r1' and 'r2' (named according
+ to the syntax explained in SPECIFYING REVISIONS above), you can ask
+ for commits that are reachable from r2 excluding those that are
reachable
+ from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
+
+The '...' (three dot) Symmetric Difference Notation::
+ A similar notation 'r1\...r2' is called symmetric difference


s/called/called the/


The wording is the original ;-) Can change.




+ of 'r1' and 'r2' and is defined as
+ 'r1 r2 --not $(git merge-base --all r1 r2)'.
+ It is the set of commits that are reachable from either one of
+ 'r1' (left side) or 'r2' (right side) but not from both.
+
+In these two shorthand notations, you can omit one end and let it
default to HEAD.
  For example, 'origin..' is a shorthand for 'origin..HEAD' and asks
"What
  did I do since I forked from the origin branch?"  Similarly,
'..origin'
  is a shorthand for 'HEAD..origin' and asks "What did the origin do
since
  I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which
is an
  empty range that is both reachable and unreachable from HEAD.

-Two other shorthands for naming a set that is formed by a commit
-and its parent commits exist.  The 'r1{caret}@' notation means all
-parents of 'r1'.  'r1{caret}!' includes commit 'r1' but excludes
-all of its parents.
+Special '{caret}' Shorthand Notations
+~~


Sorry, but this header also does not render properly in the man page.
Maybe just "Special {caret} Shorthand Notations"?  (But read on!)


rendered fine on the MYS2 man invocation - I had had to add the 
prefix to the quoted title to make it work.

What went wrong for you? (I'll read on)


Only the word "Special" is emphasized (in bold).  The rest of the header 
is plain.


I'm using asciidoc 8.6.9, so I'm likely suffering from the bug Peff 
identified.





+Two other shorthands exist, particularly useful for merge commits, is
+for naming a set that is formed by a commit and its parent commits.

-To summarize:
+The 'r1{caret}@' notation means all parents of 'r1'.
+
+'r1{caret}!' includes commit 'r1' but excludes all of its parents.


My immediate thought upon reading this is "Why not just use 'r1'?"  I
think the answer is "This truncates the range."  So, for example, "git
log r1" shows you r1 and its ancestors, while "git log r1^!" only
shows you r1.  I think you should add this example, or something similar.



I'd also asked that question in one of my replies earlier $gmane/299849.
I was then able to determine that it was a width wide 'range' covering
multi-parent situations.


Obviously, I see it more an anti-range.  You're never going to get more 
than a single commit with this notation.

Re: [PATCH v4 4/8] doc: give headings for the two and three dot notations

[Marc Branchaud]

On 2016-07-20 05:10 PM, Philip Oakley wrote:

While there, also break out the other shorthand notations and
add a title for the revision range summary (which also appears
in git-rev-parse, so keep it mixed case).

Signed-off-by: Philip Oakley 
---
  Documentation/revisions.txt | 58 -
  1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 6e9cd41..5b37283 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -242,35 +242,49 @@ specifying a single revision with the notation described 
in the
  previous section means the set of commits reachable from that
  commit, following the commit ancestry chain.

-To exclude commits reachable from a commit, a prefix '{caret}'
-notation is used.  E.g. '{caret}r1 r2' means commits reachable
-from 'r2' but exclude the ones reachable from 'r1'.
-
-This set operation appears so often that there is a shorthand
-for it.  When you have two commits 'r1' and 'r2' (named according
-to the syntax explained in SPECIFYING REVISIONS above), you can ask
-for commits that are reachable from r2 excluding those that are reachable
-from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
-
-A similar notation 'r1\...r2' is called symmetric difference
-of 'r1' and 'r2' and is defined as
-'r1 r2 --not $(git merge-base --all r1 r2)'.
-It is the set of commits that are reachable from either one of
-'r1' (left side) or 'r2' (right side) but not from both.
-
-In these two shorthands, you can omit one end and let it default to HEAD.
+Commit Exclusions
+~
+
+'{caret}' (caret) Notation::
+ To exclude commits reachable from a commit, a prefix '{caret}'
+ notation is used.  E.g. '{caret}r1 r2' means commits reachable
+ from 'r2' but exclude the ones reachable from 'r1'.
+
+Dotted Range Notations
+~~
+
+The '..' (two-dot) Range Notation::
+ The '{caret}r1 r2' set operation appears so often that there is a shorthand
+ for it.  When you have two commits 'r1' and 'r2' (named according
+ to the syntax explained in SPECIFYING REVISIONS above), you can ask
+ for commits that are reachable from r2 excluding those that are reachable
+ from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
+
+The '...' (three dot) Symmetric Difference Notation::
+ A similar notation 'r1\...r2' is called symmetric difference


s/called/called the/


+ of 'r1' and 'r2' and is defined as
+ 'r1 r2 --not $(git merge-base --all r1 r2)'.
+ It is the set of commits that are reachable from either one of
+ 'r1' (left side) or 'r2' (right side) but not from both.
+
+In these two shorthand notations, you can omit one end and let it default to 
HEAD.
  For example, 'origin..' is a shorthand for 'origin..HEAD' and asks "What
  did I do since I forked from the origin branch?"  Similarly, '..origin'
  is a shorthand for 'HEAD..origin' and asks "What did the origin do since
  I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
  empty range that is both reachable and unreachable from HEAD.

-Two other shorthands for naming a set that is formed by a commit
-and its parent commits exist.  The 'r1{caret}@' notation means all
-parents of 'r1'.  'r1{caret}!' includes commit 'r1' but excludes
-all of its parents.
+Special '{caret}' Shorthand Notations
+~~


Sorry, but this header also does not render properly in the man page. 
Maybe just "Special {caret} Shorthand Notations"?  (But read on!)



+Two other shorthands exist, particularly useful for merge commits, is
+for naming a set that is formed by a commit and its parent commits.

-To summarize:
+The 'r1{caret}@' notation means all parents of 'r1'.
+
+'r1{caret}!' includes commit 'r1' but excludes all of its parents.


My immediate thought upon reading this is "Why not just use 'r1'?"  I 
think the answer is "This truncates the range."  So, for example, "git 
log r1" shows you r1 and its ancestors, while "git log r1^!" only shows 
you r1.  I think you should add this example, or something similar.


But, really, this means that the notation is another "Commit Exclusion" 
and properly belongs in that section.


That makes this "Special Notations" section rather thin.  I suggest 
moving a slightly expanded ^@ description to a small subsection 
just before Commit Exclusions, and deleting the Special Notations 
section altogether.  So add something like this:


Commit Parents
~~

'{caret}@' Notation::
 The 'r1{caret}@' notation means all parents of 'r1',
 excluding 'r1' itself.

This smoothly re-introduces the notion of parents for readers who 
skipped to this section, and helps them make sense of the ^! notation.


Plus there's no longer anything "special" about any of the syntax.


+
+Revision Range Summary
+--


Sorry, but the man page renders this in all 

Re: [PATCH v3 4/8] doc: give headings for the two and three dot notations

[Marc Branchaud]

On 2016-07-11 04:25 PM, Philip Oakley wrote:

While there, also break out the other shorthand notations and
add a title for the revision range summary (which also appears
in git-rev-parse, so keep it mixed case).

Signed-off-by: Philip Oakley 
---
  Documentation/revisions.txt | 23 +--
  1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 79f6d03..1c59e87 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -242,35 +242,46 @@ specifying a single revision with the notation described 
in the
  previous section means the set of commits reachable from that
  commit, following the commit ancestry chain.

+The '{caret}' (caret) notation
+~~~
  To exclude commits reachable from a commit, a prefix '{caret}'
  notation is used.  E.g. '{caret}r1 r2' means commits reachable
  from 'r2' but exclude the ones reachable from 'r1'.


All of these headings render poorly in the manpage, at least for me 
(Ubuntu 16.04).  Only the first word appears in bold; the '-quoted text 
is not bold but underlined, and the rest of the header is plain.



Also, I think calling this "The ^ notation" is confusing, because 
there's already an earlier paragraph on the "^" syntax.


Maybe we don't need a header here?  I only suggest that because I'm 
having trouble coming up with a nice alternative.  "Commit Exclusion"?




-This set operation appears so often that there is a shorthand
+The '..' (two-dot) range notation
+~


Perhaps "Range notation", to mirror the capitalization of "Symmetric 
Difference" in the next header?



+The '{caret}r1 r2' set operation appears so often that there is a shorthand
  for it.  When you have two commits 'r1' and 'r2' (named according
  to the syntax explained in SPECIFYING REVISIONS above), you can ask
  for commits that are reachable from r2 excluding those that are reachable
  from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.

+The '...' (three dot) Symmetric Difference notation
+~~~
  A similar notation 'r1\...r2' is called symmetric difference
  of 'r1' and 'r2' and is defined as
  'r1 r2 --not $(git merge-base --all r1 r2)'.
  It is the set of commits that are reachable from either one of
  'r1' (Left side) or 'r2' (Right side) but not from both.

-In these two shorthands, you can omit one end and let it default to HEAD.
+In these two shorthand notations, you can omit one end and let it default to 
HEAD.
  For example, 'origin..' is a shorthand for 'origin..HEAD' and asks "What
  did I do since I forked from the origin branch?"  Similarly, '..origin'
  is a shorthand for 'HEAD..origin' and asks "What did the origin do since
  I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
  empty range that is both reachable and unreachable from HEAD.


Unfortunately the new headings make it appear that this paragraph is 
exclusively part of the '...' notation section.  Folks reading the '..' 
section are likely to skip it.


I like the examples, though.  I think it would be worthwhile to remove 
this paragraph and fold it explicitly into the '..' and '...' notation 
sections.


So add something like this to the '..' section (only the first sentence 
here is new):


Either r1 or r2 can be omitted, in which case HEAD is used as
the default.  For example, 'origin..' is a shorthand for
'origin..HEAD' and asks "What did I do since I forked from the
origin branch?"  Similarly, '..origin' is a shorthand for
'HEAD..origin' and asks "What did the origin do since I forked
from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
empty range that is both reachable and unreachable from HEAD.

And also, add the same first sentence and a different example to the 
'...' section.  Something like this:


Either r1 or r2 can be omitted, in which case HEAD is used as
the default.  For example, 'origin...' is a shorthand for
'origin...HEAD' and asks "What have I and origin both done
since I forked from the origin branch?"  Note that 'origin...'
and '...origin' ask the same question.



+Additional '{caret}' Shorthand notations
+
  Two other shorthands for naming a set that is formed by a commit
-and its parent commits exist.  The 'r1{caret}@' notation means all
-parents of 'r1'.  'r1{caret}!' includes commit 'r1' but excludes
-all of its parents.
+and its parent commits exist.


I think descriptions of ^@ and ^! should live under the main 
description of ^.  That part already describes the numeric suffix, 
so describing a couple of special suffixes there seems like a natural fit.


However, if you choose to keep this little section, you need to move the 
word "exist" earlier in the sentence:


Two other 

Re: [PATCH v3 7/8] doc: revisions - define `reachable`

[Marc Branchaud]

On 2016-07-11 04:25 PM, Philip Oakley wrote:

Do not self-define `reachable`, which can lead to misunderstanding.
Instead define `reachability` explictly.

Signed-off-by: Philip Oakley 
---
  Documentation/revisions.txt | 14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 1c59e87..a3cd28b 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -237,10 +237,16 @@ SPECIFYING RANGES
  -

  History traversing commands such as `git log` operate on a set
-of commits, not just a single commit.  To these commands,
-specifying a single revision with the notation described in the
-previous section means the set of commits reachable from that
-commit, following the commit ancestry chain.
+of commits, not just a single commit.
+
+For these commands,
+specifying a single revision, using the notation described in the
+previous section, means the `reachable` set of commits of the given
+commit.


Better as "... means the set of commits `reachable` from the given commit."


+
+A commit's reachable set is the commit itself and the commits of
+its ancestry chain.
+


s/of/in/

M.

--
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 v5 0/5] Better ref summary alignment in "git fetch"

[Marc Branchaud]

On 2016-07-01 12:03 PM, Nguyễn Thái Ngọc Duy wrote:

v5 changes the substitute symbol from '$' to '*' in compact mode and
makes sure long lines in compact mode will not make the remote ref
column too big (it's far from perfect but I think it's still good to
do).


I think the first 4 patches are great.

I have no opinion on the 5th patch, as I don't expect to use the compact 
format in any of its proposed forms (and I can't come up with an 
alternative).


M.

--
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 v3 1/6] git-fetch.txt: document fetch output

[Marc Branchaud]

On 2016-06-04 11:11 PM, Nguyễn Thái Ngọc Duy wrote:

This documents the ref update status of fetch. The structure of this
output is defined in [1]. The ouput content is refined a bit in [2]
[3] [4].

This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update).

PS. For code archaeologists, the discussion mentioned in [1] is
probably [5].

[1] 165f390 (git-fetch: more terse fetch output - 2007-11-03)
[2] 6315472 (fetch: report local storage errors ... - 2008-06-26)
[3] f360d84 (builtin-fetch: add --prune option - 2009-11-10)
[4] 0997ada (fetch: describe new refs based on where... - 2012-04-16)
[5] http://thread.gmane.org/gmane.comp.version-control.git/61657

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
  Documentation/git-fetch.txt | 46 +
  1 file changed, 46 insertions(+)

diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index efe56e0..a0d0539 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,52 @@ The latter use of the `remote..fetch` values can 
be
  overridden by giving the `--refmap=` parameter(s) on the
  command line.

+OUTPUT
+--
+
+The output of "git fetch" depends on the transport method used; this
+section describes the output when fetch over the Git protocol (either


s/fetch/fetching/


+locally or via ssh).
+
+The status of the fetch is output in tabular form, with each line
+representing the status of a single ref. Each line is of the form:
+
+---
+->  ()
+---


() should really just be , as the () are part of the 
reason string.



+
+The status of up-to-date refs is shown only if --verbose option is


s/if/if the/


Also, thanks for putting so much effort into this!

I think having a fetch.output configuration setting is perfectly fine. 
This sort of thing is really a matter of personal taste, so having 
choices is good.


M.

--
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 v2 3/3] fetch: reduce duplicate in ref update status lines

[Marc Branchaud]

On 2016-06-03 01:04 PM, Junio C Hamano wrote:

Marc Branchaud <marcn...@xiplink.com> writes:


What if we detect when the full line exceeds the terminal width, and
insert a newline after the remote ref and indent the ->  to the same
offset as its surrounding lines, like this:

  * [new branch]  2nd-index -> pclouds/2nd-index
  * [new branch]  some-kind-of-long-ref-name
-> pclouds/some-kind-of-long-ref-name
  * [new branch]  3nd-index -> pclouds/3nd-index


I am OK with this format (not in the sense that I like it better
than what the patch produces, but in the sense that I do not have
strong preference either way).  It may be hard to come up with a
good heuristics to decide where on the display width "->" should
come, though.


I think aligning it with the -> on the other lines makes the most 
aesthetic sense.


Are you worried that the right-hand-side ref might still wrap?  I'm not 
too concerned about that -- there'll always be the possibility of a ref 
name that's longer than the terminal.



+When `from` and `to` share a common suffix, the line could be
+displayed in the form:
+
+---
+   { -> } ()


If we go with this format, we'll need to document .


The sentence above calls it "a common suffix", so instead of saying
 we can say  perhaps?  Or did you mean
something more than that?


I missed that, and although I think it's an adequate description I think 
most readers will miss it too.  They eye tends to notice the 
syntax-description bits then skip down to the list of element 
descriptions to understand which bits mean what.  My brain wants to find 
"suffix" in that list.


Anyway, not a major issue, really.

M.

--
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 v2 3/3] fetch: reduce duplicate in ref update status lines

[Marc Branchaud]

On 2016-06-03 07:08 AM, Nguyễn Thái Ngọc Duy wrote:

When there are lots of ref updates, each has different name length, this
will make it easier to look because the variable part is at the end.


s/look/read/

For the record, I prefer the earlier stair-step format to this
{xxx -> yyy}/foo
stuff.

Peff suggested that a two-pass approach might not be too bad, but had 
problems when he tried it with extra-long refs.  Maybe those problems 
could be dealt with, and we could get a simple, aligned output?


What if we detect when the full line exceeds the terminal width, and 
insert a newline after the remote ref and indent the ->  to the same 
offset as its surrounding lines, like this:


 * [new branch]  2nd-index -> pclouds/2nd-index
 * [new branch]  some-kind-of-long-ref-name
   -> pclouds/some-kind-of-long-ref-name
 * [new branch]  3nd-index -> pclouds/3nd-index


---
  Documentation/git-fetch.txt |  7 +++
  builtin/fetch.c | 37 -
  t/t5510-fetch.sh|  4 ++--
  t/t5526-fetch-submodules.sh | 26 +-
  4 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 18e733c..61c3bd1 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -113,6 +113,13 @@ representing the status of a single ref. Each line is of 
the form:
  ->  ()
  ---

+When `from` and `to` share a common suffix, the line could be
+displayed in the form:
+
+---
+   { -> } ()


If we go with this format, we'll need to document .  I'm not 
sure how to describe it succinctly, especially since it's not always 
used.  Really there are two possible output formats:


  { -> } ()
   ->  ()

This is starting to feel too complex.

M.

--
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 v2 1/3] git-fetch.txt: document fetch output

[Marc Branchaud]

On 2016-06-03 07:08 AM, Nguyễn Thái Ngọc Duy wrote:

This documents the ref update status of fetch. The structure of this
output is defined in [1]. The ouput content is refined a bit in [2]


s/The ouput/The output/


[3] [4].

This patch is a copy from git-push.txt, modified a bit because the
flag '-' means different things in push (delete) and fetch (tag
update). We probably should unify the documents at some point in
future.

PS. For code archaeologists, the discussion mentioned in [1] is
probably [5].

[1] 165f390 (git-fetch: more terse fetch output - 2007-11-03)
[2] 6315472 (fetch: report local storage errors ... - 2008-06-26)
[3] f360d84 (builtin-fetch: add --prune option - 2009-11-10)
[4] 0997ada (fetch: describe new refs based on where... - 2012-04-16)
[5] http://thread.gmane.org/gmane.comp.version-control.git/61657
---
  Documentation/git-fetch.txt | 46 +
  1 file changed, 46 insertions(+)

diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index efe56e0..18e733c 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,52 @@ The latter use of the `remote..fetch` values can 
be
  overridden by giving the `--refmap=` parameter(s) on the
  command line.

+OUTPUT
+--
+
+The output of "git fetch" depends on the transport method used; this


What a mysterious statement!  Does this tabular format actually change 
when fetching over HTTP?  Maybe it's worth documenting the differences?



+section describes the output when pushing over the Git protocol


s/pushing/fetching/


+(either locally or via ssh).
+
+The status of the push is output in tabular form, with each line


s/push/fetch/


+representing the status of a single ref. Each line is of the form:
+
+---
+->  ()
+---
+
+The status of up-to-date refs is shown only if --verbose option is
+used.
+
+flag::
+   A single character indicating the status of the ref:
+(space);; for a successfully fetched fast-forward;
+`+`;; for a successful forced update;
+`x`;; for a successfully deleted ref;


I did a double-take here, until I remembered --prune.  Maybe add "(when 
using the --prune option)"?


M.

--
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] log: document the --decorate=auto option

[Marc Branchaud]

On 2016-05-27 11:56 AM, Ramsay Jones wrote:


Signed-off-by: Ramsay Jones 
---

Hi Junio,

While reading an email from Linus earlier (RFC: dynamic "auto" date formats),
I noticed that log.decorate was being set to 'auto'. Since I didn't recall
that setting (even if it's easy to guess), I went looking for the documentation 
...

This should probably be marked RFC, since I haven't checked that the formatting
is OK (I don't have the documentation toolchain installed these days).

ATB,
Ramsay Jones

  Documentation/config.txt  | 5 -
  Documentation/git-log.txt | 8 +---
  2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 53f00db..0707b3b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1956,7 +1956,10 @@ log.decorate::
command. If 'short' is specified, the ref name prefixes 'refs/heads/',
'refs/tags/' and 'refs/remotes/' will not be printed. If 'full' is
specified, the full ref name (including prefix) will be printed.
-   This is the same as the log commands '--decorate' option.
+   If 'auto' is specified, then if the output is going to a terminal,
+   the ref names are shown as if 'short' were given, otherwise no ref
+   names are shown. This is the same as the log commands '--decorate'


s/commands/command's/

M.


--
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 1/2] fetch: better alignment in ref summary

[Marc Branchaud]

On 2016-05-26 03:31 PM, Junio C Hamano wrote:

Marc Branchaud <marcn...@xiplink.com> writes:


The fact that something is buried in some odd part of the ref tree is
less relevant, IMO.  If I'm using custom fetch refspecs or other
oddities, I'll have that in the back of my head.  But what I really
care about is what ref I can use with commands like log and checkout.

So, regarding Peff's examples:


$ git fetch origin refs/pull/*/head:refs/remotes/pr/*


Just show me the "pr/foo" refs.  I know where things are coming
from. Even if I configured that fetch refspec a long time ago, I don't
need to see the exact mapping every time I fetch.


That is only because you are used to seeing them.  You cannot claim
"I'll remember forever without seeing them" without actually
experiencing not seeing them for a long time.


I don't expect (or even want) to forever remember the mapping.  It's a 
matter of context.


When fetching, I'm interested in shiny new refs and I want to work with 
them.


When configuring a remote repository, I'm interested in how to bring 
that repo's refs into my own repository.


These are two distinct modes of thought, and I don't think fetch's 
output always needs to display the latter.  Perhaps the --verbose switch 
could turn on showing how the remote refs get mapped.  I can see how 
that would occasionally be useful for debugging fetch refspecs.



I think the output should show the plain SHA values, since those are
the only things the user can use to work with those refs.


When they tell others how to reproduce what they did (or record what
they did so that they can reproduce it later), they need what it is
called at the remote end.


Which is why I included the refnames in my proposal to Peff's second 
example.  Really, the SHA and the remote's name for the SHA are the only 
meaningful things in this case.



I would hesitate to go in the approach based on discarding
information, as if it is the only way to shorten and clarify the
output.  Let's not do so before thinking things through to achieve
the same while keeping the information we give to the users.


I agree, this is not something to undertake lightly.  But I've yet to be 
convinced of the utility of always showing the ref mapping during a fetch.


I actually found fetch's output quite confusing when I first started 
using git.  It's really not obvious that the thing on the left of the 
"->" is the remote's local name, especially since to see what was 
fetched one has to use the thing on the right-hand side -- which is 
obviously in a remote-specific namespace.  (Admittedly, this was before 
checkout learned to search refs/remotes/ for things to check out.)


M.

--
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 1/2] fetch: better alignment in ref summary

[Marc Branchaud]

On 2016-05-26 01:42 PM, Junio C Hamano wrote:


True.  One of the entries in Marc's example is easily misread as
"pclouds/2nd-index branch at its refs/heads/pclouds/2nd-index was
fetched to its usual place", when Marc wanted to say "they had
2nd-index branch at refs/heads/2nd-index, and it was copied to our
refs/remotes/pclouds/2nd-index".

So even though we might be able to make sure it is unambiguous
without "this -> that" ("->" is pronounced as 'became'), it is
easily misread.


Actually, I tend to just think of it as "this is a local name you can 
use to refer to the new thing that was just fetched."  The left-hand 
side describes the thing being fetched (new or updated branch/tag/ref), 
and the right hand side shows how to locally refer to that thing.


The fact that something is buried in some odd part of the ref tree is 
less relevant, IMO.  If I'm using custom fetch refspecs or other 
oddities, I'll have that in the back of my head.  But what I really care 
about is what ref I can use with commands like log and checkout.


So, regarding Peff's examples:

> $ git fetch origin refs/pull/*/head:refs/remotes/pr/*

Just show me the "pr/foo" refs.  I know where things are coming from. 
Even if I configured that fetch refspec a long time ago, I don't need to 
see the exact mapping every time I fetch.


> $ git fetch origin refs/pull/77/head refs/pull/78/head

Ah, now this is an odd case.  Maybe there should be a different output 
format altogether for this one.  The problem is that the remote refs 
being fetched are stored without any kind of local ref.  (Commands like 
"git log FETCH_HEAD" only work with the last ref fetched, even though 
all the SHAs get added to the .git/FETCH_HEAD file.  Maybe if git 
supported a "FETCH_HEAD@{x}" syntax...)


I think the output should show the plain SHA values, since those are the 
only things the user can use to work with those refs.  Maybe something like:


From ...
 * origin:refs/pull/77/head  abcd0123
 * origin:refs/pull/78/head  453def00

(Not 100% sure about the "origin:" prefix...)

M.

--
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 1/2] fetch: better alignment in ref summary

[Marc Branchaud]

On 2016-05-22 09:59 PM, Duy Nguyen wrote:

On Mon, May 23, 2016 at 7:58 AM, Junio C Hamano  wrote:

That is, I wonder if the above can become something like:


 From github.com:pclouds/git
  * [new branch]  { -> pclouds/}2nd-index
  * [new branch]  { -> pclouds/}3nd-index
  * [new branch]  { -> pclouds/}file-watcher
  ...


The above example borrows the idea used in diffstat label for
renaming patch and I think you can design a better notataion, but a
big point is that you can shorten the whole thing by not repeating
the common part twice.  The arrow aligns merely as a side effect of
the shortening, taking advantage of the fact that most people fetch
with "$their_prefix/*:$our_prefix/*" renaming refspec.


I did think of that. My only concern is, with the new output I can't
simply copy the ref name (e.g. pclouds/2nd-index) and use it to, say,
examine with git-log. But I'm more of a "tab tab tab" person than
"copy and paste", so I don't know how often people need to do that.


Why do we need any kind of "->" at all?  How about simply (with an 
update to "old-branch" for comparison to probably-more-common output):


From github.com:pclouds/git
   cafed0c..badfeed  pclouds/old-branch
 * [new branch]  pclouds/2nd-index
 * [new branch]  pclouds/3nd-index
 * [new branch]  pclouds/file-watcher

M.

--
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: /* compiler workaround */ - what was the issue?

[Marc Branchaud]

On 2016-05-06 03:57 PM, Junio C Hamano wrote:

Marc Branchaud <marcn...@xiplink.com> writes:


On 2016-05-06 02:54 PM, Junio C Hamano wrote:


I wonder if can we come up with a short and sweet notation to remind
futhre readers that this "initialization" is not initializing but
merely squelching warnings from stupid compilers, and agree to use
it consistently?


Perhaps

#define COMPILER_UNINITIALIZED_WARNING_INITIALIZER 0

or, for short-and-sweet

#define CUWI 0

?

:)


I get that smiley.


Of course, right after I sent that I thought of

#define SPURIOUS_COMPILER_RELATED_UNINITIALIZED_WARNING_INITIALIZER 0

or

#define SCRUWI 0

Which we'd get to pronounce as "screwy".

OK, I'll shut up now.

M.

--
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: /* compiler workaround */ - what was the issue?

[Marc Branchaud]

On 2016-05-06 02:54 PM, Junio C Hamano wrote:


I wonder if can we come up with a short and sweet notation to remind
futhre readers that this "initialization" is not initializing but
merely squelching warnings from stupid compilers, and agree to use
it consistently?


Perhaps

#define COMPILER_UNINITIALIZED_WARNING_INITIALIZER 0

or, for short-and-sweet

#define CUWI 0

?

:)

M.

--
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] git-cvsserver: fix duplicate words

[Marc Branchaud]

On 2016-05-06 08:09 AM, Li Peng wrote:

Fix duplicate words in comments.

Signed-off-by: Li Peng 
---
  git-cvsserver.perl | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 02c0445..392e59e 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1156,7 +1156,7 @@ sub prepDirForOutput
  # FUTURE: This would more accurately emulate CVS by sending
  #   another copy of sticky after processing the files in that
  #   directory.  Or intermediate: perhaps send all sticky's for
-#   $seendirs after after processing all files.
+#   $seendirs after processing all files.
  }

  # update \n
@@ -2824,7 +2824,7 @@ sub statecleanup
  }

  # Return working directory CVS revision "1.X" out
-# of the the working directory "entries" state, for the given filename.
+# of the working directory "entries" state, for the given filename.
  # This is prefixed with a dash if the file is scheduled for removal
  # when it is committed.
  sub revparse
@@ -2935,7 +2935,7 @@ sub filecleanup
  return $filename;
  }

-# Remove prependdir from the path, so that is is relative to the directory
+# Remove prependdir from the path, so that is relative to the directory


This one is a typo -- it should be "it is".

Did you write a script to find these?  :)

(Also, I agree with Jeff that putting all of these changes into one 
patch would have been fine.)


M.

--
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: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Marc Branchaud]

On 2016-04-26 12:09 PM, Ævar Arnfjörð Bjarmason wrote:


Basically you can look at patches to a project on a spectrum of:

  1. You hacked something up locally
  2. It's in someone's *.git repo as a POC
  3. It's a third-party patch series used by a bunch of people
  4. In an official release but documented as experimental
  5. In an official release as a first-rate feature

Something like an experimental.WHATEVER=bool flag provides #4.


But the git project already does #4 without needing a special 
configuration tree.  In fact, you ignored the git project's "pu" branch 
entirely.  Once a feature gets onto the "next" branch, it's already much 
less experimental.  If it needs to put the word "experimental" in its 
config settings, then maybe shouldn't leave the "pu" branch in the first 
place.



I think aside from this feature just leaving these things undocumented
really provides the worst of both worlds.


Yes, I apologize.  I did not mean that things should remain 
undocumented, only that if you're afraid of users harming themselves 
then you're better off not documenting settings than labelling them as 
experimental.



Now you have some feature that's undocumented *because* you're not
sure about it, but you can't ever be sure about it unless people
actually use it, and if it's not documented at all effectively it's as
visible as some third-party patch series. I.e. only people really
involved in the project will ever use it.


Slapping "experimental" on the configuration only serves to muddy the 
waters.  Either the feature is good enough to be tried by normal users, 
or it isn't.  If it isn't ready for normal users, let it cook in pu (or 
next) for a while longer.


Git has got on just fine without having some special designation for 
not-ready-for-prime-time features, mostly because the development 
process lends itself naturally to gradually exposing things as they 
mature: topics move from the list to pu to next to master.  (The string 
"experiment" only appears 16 times in all the release notes, which I 
think is something the git project can be proud of.)


To me, designating part of the config tree as "experimental" enables 
sloppier practices, where a feature can be released with a bit less 
effort than might otherwise be acceptable, because it's clearly marked 
as experimental, and so anyone trying it out surely has the requisite 
bulletproof footwear.  (I don't mean to imply that you or any other git 
contributor might slack off on any work you do for the project. It's 
more that the ability to easily designate something as experimental 
lowers the bar a bit, and makes it more tempting to release 
not-quite-ready features.)


Far better to instead work on the feature until it's as ready as can be, 
and release it properly.


In this particular case, for example, I think your proposed approach is 
perfectly fine and does not need to be designated as "experimental" in 
any way.  With a reasonable "hooks.something" config variable to turn on 
directory support, what you've described sounds like a great feature. 
Sure, it may have bugs, it may have unintended consequences, it may not 
fulfill some odd corner cases.  But that's true for almost everything. 
As with every other git feature, it'll be developed to the best of the 
project's abilities and released.  Future patches are welcome.


M.

--
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: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Marc Branchaud]

On 2016-04-26 06:58 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> Makes sense to have an experimental.* config tree for git for stuff like this.

I disagree.

* If the point is to express some kind of warning to users, I think the
community has been much better served by leaving experimental settings
undocumented (or documented only in unmerged topic branches).  It feels like
an experimental.* tree is a doorway to putting experimental features in
official releases, which seems odd considering that (IMHO) git has so far
done very well with the carefully-planned-out integration of all sorts of
features.

* Part of the experiment is coming up with appropriate configuration knobs,
including where those knobs should live.  Often such considerations lead to a
better implementation for the feature.  Dumping things into an experimental.*
tree would merely postpone that part of the feature's design.

* Such a tree creates a flag day when the experimental feature eventually
becomes a "real" feature. That'll annoy any early adopters. Sure, they
*should* be prepared to deal with config tree bike-shedding, but still that
extra churn seems unnecessary.

M.

--
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: Tabs in commit messages - de-tabify option in strbuf_stripspace()?

[Marc Branchaud]

On 16-03-15 09:02 PM, Stefan Beller wrote:
> On Tue, Mar 15, 2016 at 6:00 PM, Stefan Beller  wrote:
>>
>> Instead of converting to whitespaces in Git, we could make use of the
>> set_tabs capability for ttys and setup the terminal for having tabs align
>> to 12,+8,+8,+8...
> 
> Or rather read in the existing tabs configuration and shift it by a constant.

Could this also help with diff output, where the leading + or - mars the
indentation in a similar way?

That opens a bit of a deeper problem, because not all the files in a single
repo necessarily use the same tab size.

M.

--
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] strtoul_ui: reject negative values

[Marc Branchaud]

On 15-09-17 11:34 AM, Matthieu Moy wrote:
> Marc Branchaud <marcn...@xiplink.com> writes:
> 
>>> --- a/git-compat-util.h
>>> +++ b/git-compat-util.h
>>> @@ -814,6 +814,9 @@ static inline int strtoul_ui(char const *s, int base, 
>>> unsigned int *result)
>>> char *p;
>>>  
>>> errno = 0;
>>> +   /* negative values would be accepted by strtoul */
>>> +   if (strchr(s, '-'))
>>> +   return -1;
>>
>> I think this is broken, in that it doesn't match strtoul's normal behaviour,
>> for strings like "1234-5678", no?
> 
> The goal here is just to read a positive integer value. Rejecting
> "1234-5678" is indeed a good thing. We already rejected it before my
> patch by checking for p (AKA endptr for strtoul), as you noted below.
> 
>> The test also doesn't work if the string has leading whitespace ("
>> -5").
> 
> Why? It rejects any string that contain the character '-', regardless of
> trailing spaces.

Right, sorry.

>>> ul = strtoul(s, , base);
>>> if (errno || *p || p == s || (unsigned int) ul != ul)
>>> return -1;
>>
>> Hmm, but we check *p here, so IIUC it's an error if the string has any
>> trailing non-digits.  Weird.
> 
> strtoul_ui is more defensive than strtoul, by design.

Fair enough, just not what I expected from a function with that name.

M.

--
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] strtoul_ui: reject negative values

[Marc Branchaud]

On 15-09-17 10:37 AM, Matthieu Moy wrote:
> strtoul_ui uses strtoul to get a long unsigned, then checks that casting
> to unsigned does not lose information and return the casted value.
> 
> On 64 bits architecture, checking that the cast does not change the value
> catches most errors, but when sizeof(int) == sizeof(long) (e.g. i386),
> the check does nothing. Unfortunately, strtoul silently accepts negative
> values, and as a result strtoul_ui("-1", ...) raised no error.
> 
> This patch catches negative values before it's too late, i.e. before
> calling strtoul. We still silently accept very large integers that wrap
> to a valid "unsigned int".
> 
> Reported-by: Max Kirillov 
> Signed-off-by: Matthieu Moy 
> ---
> So, here's a proper patch (I mean, a band-aid patch, but properly
> send ;-) ).
> 
> It should be merged before Kartik's series (or inserted at the start
> of the series) so that we get the fix before the test breakage.
> 
>  git-compat-util.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/git-compat-util.h b/git-compat-util.h
> index f649e81..1df82fa 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -814,6 +814,9 @@ static inline int strtoul_ui(char const *s, int base, 
> unsigned int *result)
>   char *p;
>  
>   errno = 0;
> + /* negative values would be accepted by strtoul */
> + if (strchr(s, '-'))
> + return -1;

I think this is broken, in that it doesn't match strtoul's normal behaviour,
for strings like "1234-5678", no?

The test also doesn't work if the string has leading whitespace ("  -5").

>   ul = strtoul(s, , base);
>   if (errno || *p || p == s || (unsigned int) ul != ul)
>   return -1;

Hmm, but we check *p here, so IIUC it's an error if the string has any
trailing non-digits.  Weird.

M.

--
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: proper remote ref namespaces

[Marc Branchaud]

On 15-08-12 02:43 AM, Jacob Keller wrote:
 Hello,
 
 Recently there was some discussion about git-notes and how we do not fetch
 notes from remotes by default. The big problem with doing so is because
 refs/remotes/* hierarchy is only setup for branches (heads), so we don't
 have any clean location to put them.
 
 Around the time of git 1.8.0, Johan Herland made a proposal for remotes to
 put all their refs in refs/remtoes/*, by moving heads into 
 refs/remotes/remoteheads/* [1]

Thanks for resurrecting this discussion.  I feel this is a fundamental
inconsistency in git's core design.  Not a fatal flaw by any means, but
something that keeps git from reaching its full potential.

 In addition, his proposal was to include remote tags into 
 refs/remotes/remote/tags and also refs/remotes/remote/replace and 
 notes similarly.
 
 During this discussion there was many people who liked the idea, and 
 others who rejected it. The main rejection reason  was two fold:
 
 (a) tags are global per project, so their namespace should be treated
 global as it is now.
 
 The proposal's counter to this is that tags aren't guaranteed to be 
 global, because today two remotes you fetch might have tags that are the
 same name with different pointers. This is currently hidden, and git
 silently picks the tag it fetched first.
 
 (b) script compatibility, as changing the ref layout  such that new git
 can't work with old repository would be bad
 
 the counter to this, is that we make git smart enough to recognize old 
 remote format, and continue to work with it. Scripts which depend on this
 layout will break, but that may not be such a huge concern.
 
 Personally, I think this proposal at least for heads, notes, replace, and
 other remote refs we'd like to pull is very useful. I don't rightly know
 the answer for tags. The linked discussion below covers several pages of
 back and forth between a few people about which method is best.
 
 I like the idea of simplifying tags and branches and notes and others to
 all fetch the same way. local stuff is in refs/heads or refs/notes and
 remote stuff is (by default) in refs/remotes/remote/tags etc
 
 But it does bring up some discussion as today we auto follow tags into
 refs/tags, and it can get weird for tags since now ambiguous tags must
 mean if there are tags of same name which point to different refs,

The tags would be disambiguated by their remote name, e.g. origin/tags/v5.6
vs. hacker/tags/v5.6.  The change would actually simplify tag auto-following.

 and we'd need to teach a bunch of logic to the ref lookup code.

Not a lot.  Existing DWIMery already handles ambiguous branches, by
preferring a local branch name over any remote ones.  The only teaching
that's really needed is to resolve shorthand like origin/v5.6 to
refs/remotes/origin/tags/v5.6 (i.e. to look for anything matching
refs/remotes/origin/*/v5.6) but that doesn't seem very difficult.

There is the question of how the user can even know if there's ambiguity.
Aside from paying attention to git fetch output, I think we could extend
git tag (and git branch) in the case where the user specifies an existing
tag (or branch).  Right now both commands fail because the name already
exists.  All we need to do is extend that error message a bit, e.g.:

 git tag v5.6
fatal: tag 'next' already exists as
v5.6 - a3a0e5d67d554a685abd897bc3ce4ffa4e74c812
origin/v5.6 remoteX/v5.6 - 504346bbf9b02387b51f232f4db9c1860789b135
hacker/v5.6 - fb4aa3533f81700789b3fb119e527410678e8d8d

Here we see that our local v5.6, and hacker's v5.6, are unique, but origin
and remoteX both have the same v5.6.

Well, same-ish.  I think those SHA IDs should be the things the tags point
at, not the tag objects' IDs.  This keeps things consistent between
lightweight and annotated/signed tags.  It does mean though that origin/v5.6
and remoteX/v5.6 might be different tag objects that happen to point at the
same thing.  I'm not sure the distinction is all that germane, and it's
something that the user could disambiguate with git tag -v or git show.

 I am looking at ways to help git-notes be easier to use, so that we by 
 default fetch notes, and enable easier merge, since we'd have default 
 locations to merge from and to. This would make the sharing of notes a lot
 easier, which is one of their primary sticking points.. you can't really
 share them without *everyone* working to do it the same way you do. By
 making a default policy, sharing becomes natural, and users can easily add
 *public* notes to commits for things such as bug ids and other things
 which are not discovered until after the commit is created.
 
 In addition, the easy ability to share replaces might also be helpful, 
 though IMHO not as valuable as git-notes.
 
 I think that the only logical refs layout is 
 refs/remotes/remote/(heads|tags|notes|replace)
 
 and adding refs/remote-notes and refs/remote-replace is not really a
 clean solution.
 
 Given 

Re: A good time to pull from your gitk tree?

[Marc Branchaud]

On 15-03-24 07:06 PM, Paul Mackerras wrote:
 On Mon, Mar 23, 2015 at 12:03:37PM -0700, Junio C Hamano wrote:

 Is it a good time for me to pull from you, or do you recommend me to
 wait for a bit, expecting more?  We'll go in the pre-release freeze
 soon-ish, so I thought I should ping.
 
 Now is a good time to pull from the usual place, thanks.

Hi Paul,

Is the usual place still
git://ozlabs.org/~paulus/gitk
?

The latest commit I get from there is

commit c846920f23704ece225cc5b6c7566777fb561502
Author: Paul Mackerras pau...@samba.org
Date:   Sun Mar 15 17:25:02 2015 +1100

gitk: Update .po files

Signed-off-by: Paul Mackerras pau...@samba.org

It seems to be missing some of the changes you accepted earlier this week.

Thanks!

M.

--
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] gitk: Use translated version of Command line in getcommitlines.

[Marc Branchaud]

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---

I noticed this today.  I think this change is needed for getcommitlines to
work properly with translated gitk's.

M.

 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 90419e3..fd5b50a 100755
--- a/gitk
+++ b/gitk
@@ -1442,7 +1442,7 @@ proc getcommitlines {fd inst view updating}  {
if {[string range $err 0 4] == usage} {
set err Gitk: error reading commits$fv:\
bad arguments to git log.
-   if {$viewname($view) eq Command line} {
+   if {$viewname($view) eq [mc Command line]} {
append err \
  (Note: arguments to gitk are passed to git log\
 to allow selection of commits to be displayed.)
-- 
2.3.5

--
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

[PATCHv2] gitk: Show the current view's name in the window title.

[Marc Branchaud]

If the current view is the Command line view, show the command line
arguments instead of the view name.

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---

This is v2 of my previous Show the command-line revs in the window title RFC
patch.  (I'm having trouble accessing gmane, or I'd include a link here.)

This version incorporates Paul's feedback (thanks!) and handles view properly.

M.

 gitk | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/gitk b/gitk
index b859879..90419e3 100755
--- a/gitk
+++ b/gitk
@@ -4034,6 +4034,19 @@ proc shellsplit {str} {
 return $l
 }
 
+proc set_window_title {} {
+global appname curview viewname vrevs
+set rev [mc All files]
+if {$curview ne 0} {
+   if {$viewname($curview) eq [mc Command line]} {
+   set rev [string map {--gitk-symmetric-diff-marker --merge} 
$vrevs($curview)]
+   } else {
+   set rev $viewname($curview)
+   }
+}
+wm title . [reponame]: $rev - $appname
+}
+
 # Code to implement multiple views
 
 proc newview {ishighlight} {
@@ -4510,6 +4523,7 @@ proc showview {n} {
 } elseif {$numcommits == 0} {
show_status [mc No commits selected]
 }
+set_window_title
 }
 
 # Stuff relating to the highlighting facility
@@ -6650,6 +6664,7 @@ proc show_status {msg} {
 global canv fgcolor
 
 clear_display
+set_window_title
 $canv create text 3 3 -anchor nw -text $msg -font mainfont \
-tags text -fill $fgcolor
 }
@@ -12393,7 +12408,7 @@ catch {
 }
 # wait for the window to become visible
 tkwait visibility .
-wm title . [reponame] - $appname
+set_window_title
 update
 readrefs
 
-- 
2.3.5

--
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: A good time to pull from your gitk tree?

[Marc Branchaud]

On 15-03-24 07:06 PM, Paul Mackerras wrote:
 On Mon, Mar 23, 2015 at 12:03:37PM -0700, Junio C Hamano wrote:

 Is it a good time for me to pull from you, or do you recommend me to
 wait for a bit, expecting more?  We'll go in the pre-release freeze
 soon-ish, so I thought I should ping.
 
 Now is a good time to pull from the usual place, thanks.

Paul, could you react to the gitk window-title patches I've posted:

http://news.gmane.org/find-root.php?group=gmane.comp.version-control.gitarticle=262080

Thanks!

M.

--
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/2] gitk: Rearrange window title to be more conventional.

[Marc Branchaud]

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---

I did a bit of googling but couldn't find some standard that says the
application name goes at the end of the title bar.  But this is how all the
browsers and other apps I use regularly do things.

M.

 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 9a2daf3..b859879 100755
--- a/gitk
+++ b/gitk
@@ -12393,7 +12393,7 @@ catch {
 }
 # wait for the window to become visible
 tkwait visibility .
-wm title . $appname: [reponame]
+wm title . [reponame] - $appname
 update
 readrefs
 
-- 
2.3.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 2/2] gitk: Show the rev(s) the user specified on the command line in the window title.

[Marc Branchaud]

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---

I often open multiple gitk windows in the same working directory to examine
other branches or refs in the repo.  This change allows me to distinguish
which window is showing what.

M.

 gitk | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gitk b/gitk
index b859879..379b13a 100755
--- a/gitk
+++ b/gitk
@@ -488,7 +488,7 @@ proc reset_pending_select {selid} {
 }
 
 proc getcommits {selid} {
-global canv curview need_redisplay viewactive
+global appname canv curview need_redisplay viewactive vrevs
 
 initlayout
 if {[start_rev_list $curview]} {
@@ -498,6 +498,11 @@ proc getcommits {selid} {
 } else {
show_status [mc No commits selected]
 }
+set rev $vrevs($curview)
+if {$rev eq } {
+   set rev HEAD
+}
+wm title . [reponame]: $rev - $appname
 }
 
 proc updatecommits {} {
-- 
2.3.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

Re: [PATCH 0/2] Tweaking the gitk window title.

[Marc Branchaud]

On 15-01-06 12:51 PM, Marc Branchaud wrote:
 The first patch simply changes the title from gitk: dir to dir - gitk,
 which is the title layout used by most of the applications on my Kubuntu box.
 
 The second patch is the one that I'm more keen to see accepted.  It relies
 on the first only in that it follows the new title layout.

Ping?

M.

--
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: [RFC/PATCH] branch: name detached HEAD analogous to status

[Marc Branchaud]

On 15-02-22 02:21 PM, Junio C Hamano wrote:
 Michael J Gruber g...@drmicha.warpmail.net writes:
 
 git status carefully names a detached HEAD at resp. from a rev or
 ref depending on whether the detached HEAD has moved since. git branch
 always uses from, which can be confusing, because a status-aware user
 would interpret this as moved detached HEAD.

 Make git branch use the same logic and wording.
 
 Yeah, otherwise the user would wonder why sometimes the object name
 after that from matches git rev-parse HEAD and sometimes does
 not.
 
 In order to make sure that it will be easy for us to maintain that
 these two commands will keep using the same logic and wording after
 this fix is applied, should this patch do a bit more?  Or is it
 worth doing that for such a small piece of code to be shared?
 
 The following is a tangent and I do not think it is likely we would
 do anything about it, but I wonder what value we give the end users
 by showing the from information, both in status and branch in
 the first place.  When I am on a detached HEAD, I'd be doing one of
 these three things:
 
  (1) I am on some kind of sequencing machinery (e.g. rebase -i,
  cherry-pick A..B, or bisect).  It does not matter to me at
  all if I am at the same commit at which I started the sequenced
  operations or the sequencing machinery has moved me one or more
  commits along its planned course of action, or where the
  original point the sequencing machinery detached the HEAD at.
  I suspect that I would not use git status or git branch in
  this mode anyway.
 
  (2) I am sight-seeing, starting with e.g. git checkout v2.0.0,
  and moving around with git checkout $some_other_commit.  I'd
  always see that I am at the commit I last checked out, so the
  distinctions would not be even shown to me.
 
  (3) I am experimenting to fix or enhance an existing thing that is
  meant to eventually hit a concrete branch, but I do not know if
  the experiment would pan out. git checkout $topic~$n would be
  to start from near the tip of that $topic ($n may often be 0
  but not always) and then I would git commit my experiments.
  When I assess my progress, I'd be interested in what I have
  that is not in $topic and vice versa since I started that
  experiment, so

I find it very useful, because I often work on HEADs detached from remote
branches (git checkout origin/foo).  If I'm sight-seeing, I like the
reminder of which remote branch I checked out, especially because I often
have several working tress going at the same time.  I also often make trivial
changes, like typo fixes, on such detached HEADs, and here too I appreciate
the reminder of which remote branch I should push HEAD to.

M.

--
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: [RFC/PATCH] branch: name detached HEAD analogous to status

[Marc Branchaud]

On 15-02-22 12:38 PM, Michael J Gruber wrote:
 git status carefully names a detached HEAD at resp. from a rev or
 ref depending on whether the detached HEAD has moved since. git branch
 always uses from, which can be confusing, because a status-aware user
 would interpret this as moved detached HEAD.
 
 Make git branch use the same logic and wording.

Except that the wording in git branch is more correct, especially if the
detached HEAD contains new commits.

In other words, at is only correct if the detached HEAD matches the ref.
If the HEAD has other commits, it is no longer at that ref but instead it
has grown from it.

But even if the detached HEAD matches the ref, saying it came from that ref
(with 0 extra commits) is still better than saying
detached-HEAD-with-extra-commits is at the ref.

M.

--
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: [RFC/PATCH] branch: name detached HEAD analogous to status

[Marc Branchaud]

On 15-02-23 11:24 AM, Michael J Gruber wrote:
 Marc Branchaud venit, vidit, dixit 23.02.2015 16:12:
 On 15-02-22 12:38 PM, Michael J Gruber wrote:
 git status carefully names a detached HEAD at resp. from a rev or
 ref depending on whether the detached HEAD has moved since. git branch
 always uses from, which can be confusing, because a status-aware user
 would interpret this as moved detached HEAD.

 Make git branch use the same logic and wording.

 Except that the wording in git branch is more correct, especially if the
 detached HEAD contains new commits.

 In other words, at is only correct if the detached HEAD matches the ref.
 If the HEAD has other commits, it is no longer at that ref but instead it
 has grown from it.
 
 Sure, but that's exactly what git status does. Haven't you tried out?
 
 And it's exactly what I suggest for git branch. It conveys more information.

Oops, right.  Sorry, I got blinded by the various detached at examples in
your patch's notes.

M.

--
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 0/2] Tweaking the gitk window title.

[Marc Branchaud]

On 15-01-06 12:51 PM, Marc Branchaud wrote:
 The first patch simply changes the title from gitk: dir to dir - gitk,
 which is the title layout used by most of the applications on my Kubuntu box.
 
 The second patch is the one that I'm more keen to see accepted.  It relies
 on the first only in that it follows the new title layout.

Ping?

M.

--
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/2] gitk: Rearrange window title to be more conventional.

[Marc Branchaud]

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---

I did a bit of googling but couldn't find some standard that says the
application name goes at the end of the title bar.  But this is how all the
browsers and other apps I use regularly do things.

M.

 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 78358a7..03de545 100755
--- a/gitk
+++ b/gitk
@@ -12293,7 +12293,7 @@ catch {
 }
 # wait for the window to become visible
 tkwait visibility .
-wm title . $appname: [reponame]
+wm title . [reponame] - $appname
 update
 readrefs
 
-- 
2.2.1

--
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 PATCH 2/2] gitk: Show the rev(s) the user specified on the command line in the window title.

[Marc Branchaud]

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---

I often open multiple gitk windows in the same working directory to examine
other branches or refs in the repo.  This change allows me to distinguish
which window is showing what.

This is an RFC because it doesn't behave great with views.  I don't use views
at all, so this doesn't bother me.  I'm not too sure what should be displayed
if the user changes views -- probably the view name, but I'm not sure how to
get a that in the code.

M.

 gitk | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 03de545..b8463fd 100755
--- a/gitk
+++ b/gitk
@@ -488,7 +488,7 @@ proc reset_pending_select {selid} {
 }
 
 proc getcommits {selid} {
-global canv curview need_redisplay viewactive
+global appname canv curview need_redisplay viewactive vrevs
 
 initlayout
 if {[start_rev_list $curview]} {
@@ -498,6 +498,11 @@ proc getcommits {selid} {
 } else {
show_status [mc No commits selected]
 }
+set rev $vrevs($curview)
+if {$rev eq } {
+   set rev HEAD
+}
+wm title . [reponame]: $rev - $appname
 }
 
 proc updatecommits {} {
-- 
2.2.1

--
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 0/2] Tweaking the gitk window title.

[Marc Branchaud]

The first patch simply changes the title from gitk: dir to dir - gitk,
which is the title layout used by most of the applications on my Kubuntu box.

The second patch is the one that I'm more keen to see accepted.  It relies
on the first only in that it follows the new title layout.

M.

--
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: Our cumbersome mailing list workflow

[Marc Branchaud]

On 14-11-28 09:31 AM, Michael Haggerty wrote:
 On 11/27/2014 06:46 PM, Torsten Bögershausen wrote:
 On 2014-11-25 01.28, Michael Haggerty wrote:
 []
 Let me list the aspects of our mailing list workflow that I find
 cumbersome as a contributor and reviewer:

 * Submitting patches to the mailing list is an ordeal of configuring
 format-patch and send-email and getting everything just right, using
 instructions that depend on the local environment.
 Typically everything fits into ~/.gitconfig,
 which can be carried around on a USB-Stick.
 Is there any details which I miss, or howtows we can improve ?
 
 I used to need one setup at work and a different one at home (because of
 how my email was configured), and sometimes had to switch back and forth
 as I carried my notebook around.
 
 [...]
   * I do git fetch gitster, then try to figure out whether the branch
 I'm interested in is present, what its name is, and whether the version
 in your tree is the latest version, then git checkout xy/foobar.
 There are 12 branches from mh/, so it should be possible to find the name,
 und run git log gitster/xy/fix_this_bug or so.
 Even more important, this branch is the single point of truth, because
 this branch may be merged eventually, and nothing else.
 
 I know it's *possible*. The question is whether it could be made easier.
 
 * Following patch series across iterations is also awkward. To compare
 two versions, I have to first get both patch series into my repo, which
 involves digging through the ML history to find older versions, followed
 by the git am steps. Often submitters are nice enough to put links to
 previous versions of their patch series in their cover letters, but the
 links are to a web-based email archive, from which it is even more
 awkward to grab and apply patches. So in practice I then go back to my
 email client and search my local archive for my copy of the same email
 that was referenced in the archive, and apply the patch from there.
 Finding comments about old versions of a patch series is nearly as much
 work.
 In short:
 We can ask every contributor, if the patch send to the mailing list
 is available on a public Git-repo, and what the branch name is,
 like _V2.. Does this makes sense ?
 
 That would be helpful, but it would put yet *another* requirement on the
 submitter (to send patch emails *and* push the branch to some accessible
 repository). We regulars could script this pretty easily, but people who
 only contribute occasionally or who are trying to get started will be
 even more overwhelmed.

A bot could subscribe to the list and create branches in a public repo.
(This idea feels familiar -- didn't somebody attempt this already?)

Integrate the bot into the list manager, and every PATCH email sent through
the list could have the patch's URL (maybe in the footer, or as an X- header).

Could this make a decent GSoC project?

M.

--
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] RelNotes: Spelling grammar tweaks.

[Marc Branchaud]

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
 Documentation/RelNotes/2.2.0.txt | 102 +++
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/Documentation/RelNotes/2.2.0.txt b/Documentation/RelNotes/2.2.0.txt
index d4001c5..9d9d5d5 100644
--- a/Documentation/RelNotes/2.2.0.txt
+++ b/Documentation/RelNotes/2.2.0.txt
@@ -9,20 +9,20 @@ Ports
  * Building on older MacOS X systems automatically sets
the necessary NO_APPLE_COMMON_CRYPTO build-time option.
 
- * The support to build with NO_PTHREADS has been resurrected.
+ * Building with NO_PTHREADS has been resurrected.
 
- * Compilation options has been updated a bit to support z/OS port
-   better.
+ * Compilation options have been updated a bit to better support the
+   z/OS port.
 
 
 UI, Workflows  Features
 
- * git archive learned to filter what gets archived with pathspec.
+ * git archive learned to filter what gets archived with a pathspec.
 
  * git config --edit --global starts from a skeletal per-user
configuration file contents, instead of a total blank, when the
-   user does not already have any.  This immediately reduces the
-   need for a later Have you forgotten setting core.user? and we
+   user does not already have any global config.  This immediately reduces the
+   need later Have you forgotten to set core.user? messages, and we
can add more to the template as we gain more experience.
 
  * git stash list -p used to be almost always a no-op because each
@@ -33,35 +33,35 @@ UI, Workflows  Features
  * Sometimes users want to report a bug they experience on their
repository, but they are not at liberty to share the contents of
the repository.  fast-export was taught an --anonymize option
-   to replace blob contents, names of people and paths and log
+   to replace blob contents, names of people, paths and log
messages with bland and simple strings to help them.
 
  * git difftool learned an option to stop feeding paths to the
diff backend when it exits with a non-zero status.
 
- * git grep allows to paint (or not paint) partial matches on
+ * git grep learned to paint (or not paint) partial matches on
context lines when showing grep -Cnum output in color.
 
- * log --date=iso uses a slight variant of ISO 8601 format that is
-   made more human readable.  A new --date=iso-strict option gives
-   datetime output that is more strictly conformant.
+ * log --date=iso uses a slight variant of the ISO 8601 format that is
+   more human readable.  A new --date=iso-strict option gives
+   datetime output that conforms more strictly.
 
  * The logic git prune uses is more resilient against various corner
cases.
 
  * A broken reimplementation of Git could write an invalid index that
-   records both stage #0 and higher stage entries for the same path.
+   records both stage #0 and higher-stage entries for the same path.
We now notice and reject such an index, as there is no sensible
fallback (we do not know if the broken tool wanted to resolve and
-   forgot to remove higher stage entries, or if it wanted to unresolve
-   and forgot to remove the stage#0 entry).
+   forgot to remove the higher-stage entries, or if it wanted to unresolve
+   and forgot to remove the stage #0 entry).
 
- * The temporary files git mergetool uses are named to avoid too
+ * The temporary files git mergetool uses are renamed to avoid too
many dots in them (e.g. a temporary file for hello.c used to be
named e.g. hello.BASE.4321.c but now uses underscore instead,
e.g. hello_BASE_4321.c).
 
- * The temporary files git mergetools uses can be placed in a newly
+ * The temporary files git mergetool uses can be placed in a newly
created temporary directory, instead of the current directory, by
setting the mergetool.writeToTemp configuration variable.
 
@@ -73,7 +73,7 @@ UI, Workflows  Features
to consume their input fully (not following this requirement used
to result in intermittent errors in git push).
 
- * The pretty-format specifier %d, which expanded to  (tagname)
+ * The pretty-format specifier %d, which expands to  (tagname)
for a tagged commit, gained a cousin %D that just gives the
tagname without frills.
 
@@ -86,14 +86,14 @@ UI, Workflows  Features
without having to trust the server.
 
  * git interpret-trailers is a new filter to programmatically edit
-the tail end of the commit log messages.
+   the tail end of the commit log messages.
 
  * git help everyday shows the Everyday Git in 20 commands or so
document, whose contents have been updated to more modern Git
practice.
 
- * On the git svn front, work to reduce memory consumption and
-   to improve handling of mergeinfo progresses.
+ * On the git svn front, work progresses to reduce memory consumption and
+   to improve handling of mergeinfo.
 
 
 Performance, Internal Implementation, etc.
@@ -106,18 +106,18 @@ Performance, Internal Implementation, etc