Re: [PATCH] completion: suggest sequencer commands for revert

2015-05-29 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
>
> >  contrib/completion/git-completion.bash | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/contrib/completion/git-completion.bash 
> > b/contrib/completion/git-completion.bash
> > index bfc74e9..3c00acd 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -2282,6 +2282,11 @@ _git_reset ()
> >
> >  _git_revert ()
> >  {
> > + local dir="$(__gitdir)"
> > + if [ -f "$dir"/REVERT_HEAD ]; then
> > + __gitcomp "--continue --quit --abort"
> > + return
> > + fi
> >   case "$cur" in
> >   --*)
> >   __gitcomp "--edit --mainline --no-edit --no-commit --signoff"

This corresponds exactly to what we do for git-cherry-pick:

local dir="$(__gitdir)"
if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
__gitcomp "--continue --quit --abort"
return
fi

Perhaps _git_revert() and _git_cherry_pick() should call into the same
function with different arguments.

This looks fine though.
--
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: [PATCHv4] Documentation: triangular workflow

2016-06-11 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> Jordan DE GEA wrote:
>
> > +* Allows contributors to work with Git even though they do not have
> > +write access to **UPSTREAM**.
> >
> > +* Allows maintainers to receive code from contributors they may not
> > +trust.

Triangular workflow is the ability to accept changes from contributors
without mailing patches back-and-forth. Whether they send a pull
request or commit directly to the master repository when review is
done, is inconsequential. Essentially, they maintain forks of
upstream, which they work on at their own pace.

> > +* Code review is more efficient
>
> I have no idea what data you have to back this claim up.  More
> efficient compared to what?

They're orthogonal. LLVM has one giant SVN server that everyone
commits directly to. However, they review process is a lot more
efficient than GitHub projects, because they use Phabricator. What
does code review tool have to do with triangular workflow?

> > +Preparation
> > +~~~
> > +
> > +Cloning from **PUBLISH**, which is a fork of **UPSTREAM** or an empty
> > +repository.
> > +
> > +==
> > +`git clone `
> > +==
> > +
> > +Setting the behavior of push for the triangular workflow:
> > +
> > +===
> > +`git config push.default current`
> > +===
> > +
> > +Adding **UPSTREAM** remote:
> > +
> > +===
> > +`git remote add upstream `
> > +===
> > +
> > +With the `remote add` above, using `git pull upstream` pulls there,
> > +instead of saying its URL. In addition, `git pull` can pull from
> > +**UPSTREAM** without argument.
> > +
> > +For each branch requiring a triangular workflow, set
> > +`branch..remote` and `branch..pushRemote`.
> > +
> > +Example with master as :
> > +===
> > +* `git config branch.master.remote upstream`
> > +* `git config branch.master.pushRemote origin`
> > +===

It's much too simple now. Just `git clone `, `git remote add
mine `, and `git config remote.pushdefault mine`. Only the
last line requires an explanation.

> Instead you would set default.pushRemote to publish
> just once, and no matter how many branches you create later, you do
> not have to do anything special.

I think you meant remote.pushdefault here?
--
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] send-email: new option to walkaround email server limits

2017-05-03 Thread Ramkumar Ramachandra
For the list, in plain text:

IIUC, they use the date received to sort. I think this might stem from
a historical cruft: emails sometimes took non-trivial amounts of time
to transit, back in the old days. MUAs (especially web-based ones)
probably did not want to violate user expectation by placing a new
email under several already-read emails. I would argue that this was
reasonable behavior. Further, since email is near-instantaneous today,
it really makes no difference which way the MUA sorts. Except for git
send-email.

It might be acceptable to put in a "practical hack" to help out MUAs
in the context of "near instant forwarding". I would argue against it
on grounds of it being an ugly hack, for very little benefit. The
patch that began this thread is also ugly, but has the important
consequence of enabling some people to use git send-email at all.

p.s- We should probably allow emails from mobile devices on the list.
It usually contains a HTML subpart.


Re: Git GSoC 2014

2014-02-13 Thread Ramkumar Ramachandra
Jeff King wrote:
>   - ideas ideas ideas

I'll throw in a few ideas from half-finished work.

1. Speed up git-rebase--am.sh

Currently, git-rebase--am.sh is really slow because it dumps each
patch to a file using git-format-patch, and picks it up to apply
subsequently using git-am. Find a way to speed this up, without
sacrificing safety. You can use the continuation features of
cherry-pick, and dump to file only to persist state in the case of a
failure.

Language: Shell script, C
Difficulty: Most of the difficulty lies in "what to do", not so much
"how to do it". Might require modifying cherry-pick to do additional
work on failure.

2. Invent new conflict style

As an alternative to the diff3 conflict style, invent a conflict style
that shows the original unpatched segment along with the raw patch
text. The user can then apply the patch by hand.

Language: C
Difficulty: Since it was first written, very few people have touched
the xdiff portion of the code. Since the area is very core to git, the
series will have to go through a ton of iterations.

3. Rewrite git-branch to use git-for-each-ref

For higher flexibility in command-line options and output format, use
git for-each-ref to re-implement git-branch. The first task is to grow
features that are in branch but not fer into fer (like --column,
--merged, --contains). The second task is to refactor fer so that an
external program can call into it.

Language: C
Difficulty: fer was never written with the idea of being reusable; it
therefore persists a lot of global state, and even leaks memory in
some places. Refactoring it to be more modern is definitely a
challenge.

4. Implement @{publish}
(I just can't find the time to finish this)

@{publish} is a feature like @{upstream}, showing the state of the
publish-point in the case of triangular workflows. Implement this
while sharing code with git-push, and polish it until the prompt shows
publish-state.

Language: C, Shell script
Difficulty: Once you figure out how to share code with git-push, this
task should be relatively straightforward.
--
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: Git GSoC 2014

2014-02-14 Thread Ramkumar Ramachandra
Jeff King wrote:
>> 1. Speed up git-rebase--am.sh
>
> Isn't the merge backend faster? I thought that was the point of it.

I suppose, but I thought git-rebase--am.sh (the default flavor) could
be improved by leveraging relatively new cherry-pick features; I
assumed that the reason it was using format-patch/ am was because it
was written before cherry-pick matured. Alternatively, can you think
of a project that involves working on the sequencer?

>> 3. Rewrite git-branch to use git-for-each-ref
>
> I actually have this about 95% done, waiting for the patches to be
> polished. So I don't think it makes a good GSoC project (it would be
> stupid to start from scratch, and polishing my patches is a lame
> project).

Oh. I look forward to using a nicer git-branch soon.

>> 4. Implement @{publish}
>
> I think this could be a good GSoC-sized topic. I'm going to adjust the
> title to be "better support for triangular workflows". I think they may
> want to examine other issues in the area, rather than drilling down on
> @{publish} in particular (but ultimately, it is up to the student to
> propose what they want to do).

That makes the project a little more open-ended then. I like it.

I was hoping you'd have more comments on "3. Invent new conflict
style". Although I'm not sure the conflict style I proposed would be
terribly useful in the general case, it'll give the student an
opportunity to look at older/ lesser-known portions of the codebase.
--
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: What's cooking in git.git (Feb 2014, #06; Wed, 19)

2014-02-21 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> * jk/branch-at-publish-rebased (2014-01-17) 5 commits
>  - t1507 (rev-parse-upstream): fix typo in test title
>  - implement @{publish} shorthand
>  - branch_get: provide per-branch pushremote pointers
>  - branch_get: return early on error
>  - sha1_name: refactor upstream_mark
>
>  Give an easier access to the tracking branches from "other" side in
>  a triangular workflow by introducing B@{publish} that works in a
>  similar way to how B@{upstream} does.
>
>  Meant to be used as a basis for whatever Ram wants to build on.
>
>  Will hold.

Since I've decided that I don't have the time to work on this, we've
added this to the list of project ideas for GSoC 2014
(http://git.github.io/SoC-2014-Ideas.html). Hopefully, a student will
come along and volunteer to finish this.
--
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] remote: handle pushremote config in any order order

2014-02-24 Thread Ramkumar Ramachandra
Jeff King wrote:
> Thus, config like:
>
>   [branch "master"]
>   pushremote = foo
>   [remote]
>   pushdefault = bar
>
> erroneously ends up pushing to "bar" from the master branch.

Oh, ouch. Thanks for fixing this.
--
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] Documentation/merge-strategies: avoid hyphenated commands

2014-03-16 Thread Ramkumar Ramachandra
Replace git-pull and git-merge with the corresponding un-hyphenated
versions. While at it, use ` to mark it up instead of '.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/merge-strategies.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/merge-strategies.txt 
b/Documentation/merge-strategies.txt
index 12133b9..19f3a5d 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -1,10 +1,10 @@
 MERGE STRATEGIES
 
 
-The merge mechanism ('git-merge' and 'git-pull' commands) allows the
+The merge mechanism (`git merge` and `git pull` commands) allows the
 backend 'merge strategies' to be chosen with `-s` option.  Some strategies
 can also take their own options, which can be passed by giving `-X`
-arguments to 'git-merge' and/or 'git-pull'.
+arguments to `git merge` and/or `git pull`.
 
 resolve::
This can only resolve two heads (i.e. the current branch
-- 
1.9.0.431.g014438b

--
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 pu] Documentation/giteveryday: fix some obvious problems

2014-03-16 Thread Ramkumar Ramachandra
Fix a few minor things.

Signed-off-by: Ramkumar Ramachandra 
---
 Philip,

 I spotted a few obvious issues with your giteveryday patch in
 pu. Maybe Junio can squash this into your patch? Contents are still a
 bit stale, but I'm not sure what other markup problems are there.

 Documentation/giteveryday.txt | 78 +--
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/Documentation/giteveryday.txt b/Documentation/giteveryday.txt
index 8dc298f..82ff8ec 100644
--- a/Documentation/giteveryday.txt
+++ b/Documentation/giteveryday.txt
@@ -35,8 +35,6 @@ following commands.
 
   * linkgit:git-init[1] to create a new repository.
 
-  * linkgit:git-show-branch[1] to see where you are.
-
   * linkgit:git-log[1] to see what happened.
 
   * linkgit:git-checkout[1] and linkgit:git-branch[1] to switch
@@ -61,8 +59,8 @@ following commands.
 Examples
 
 
-Use a tarball as a starting point for a new repository.::
-+
+Use a tarball as a starting point for a new repository:
+
 
 $ tar zxf frotz.tar.gz
 $ cd frotz
@@ -71,12 +69,12 @@ $ git add . <1>
 $ git commit -m "import of frotz source tree."
 $ git tag v2.43 <2>
 
-+
+
 <1> add everything under the current directory.
 <2> make a lightweight, unannotated tag.
 
-Create a topic branch and develop.::
-+
+Create a topic branch and develop:
+
 
 $ git checkout -b alsa-audio <1>
 $ edit/compile/test
@@ -95,7 +93,7 @@ $ git merge alsa-audio <10>
 $ git log --since='3 days ago' <11>
 $ git log v2.43.. curses/ <12>
 
-+
+
 <1> create a new topic branch.
 <2> revert your botched changes in `curses/ux_audio_oss.c`.
 <3> you need to tell Git if you added a new file; removal and
@@ -137,8 +135,8 @@ addition to the ones needed by a standalone developer.
 Examples
 
 
-Clone the upstream and work on it.  Feed changes to upstream.::
-+
+Clone the upstream and work on it.  Feed changes to upstream:
+
 
 $ git clone git://git.kernel.org/pub/scm/.../torvalds/linux-2.6 my2.6
 $ cd my2.6
@@ -151,7 +149,7 @@ $ git reset --hard ORIG_HEAD <6>
 $ git gc <7>
 $ git fetch --tags <8>
 
-+
+
 <1> repeat as needed.
 <2> extract patches from your branch for e-mail submission.
 <3> `git pull` fetches from `origin` by default and merges into the
@@ -166,8 +164,8 @@ area we are interested in.
 and store them under `.git/refs/tags/`.
 
 
-Push into another repository.::
-+
+Push into another repository:
+
 
 satellite$ git clone mothership:frotz frotz <1>
 satellite$ cd frotz
@@ -185,7 +183,7 @@ mothership$ cd frotz
 mothership$ git checkout master
 mothership$ git merge satellite/master <5>
 
-+
+
 <1> mothership machine has a frotz repository under your home
 directory; clone from it to start a repository on the satellite
 machine.
@@ -200,8 +198,8 @@ as a back-up method.
 <5> on mothership machine, merge the work done on the satellite
 machine into the master branch.
 
-Branch off of a specific tag.::
-+
+Branch off of a specific tag:
+
 
 $ git checkout -b private2.6.14 v2.6.14 <1>
 $ edit/compile/test; git commit -a
@@ -209,7 +207,7 @@ $ git checkout master
 $ git format-patch -k -m --stdout v2.6.14..private2.6.14 |
   git am -3 -k <2>
 
-+
+
 <1> create a private branch based on a well known (but somewhat behind)
 tag.
 <2> forward port all changes in `private2.6.14` branch to `master` branch
@@ -240,8 +238,8 @@ commands in addition to the ones needed by participants.
 Examples
 
 
-My typical Git day.::
-+
+My typical Git day:
+
 
 $ git status <1>
 $ git show-branch <2>
@@ -261,10 +259,10 @@ $ git cherry-pick master~4 <9>
 $ compile/test
 $ git tag -s -m "GIT 0.99.9x" v0.99.9x <10>
 $ git fetch ko && git show-branch master maint 'tags/ko-*' <11>
-$ git push ko <12>
-$ git push ko v0.99.9x <13>
+$ git push ko
+$ git push ko v0.99.9x
 
-+
+
 <1> see what I was in the middle of doing, if any.
 <2> see what topic branches I have and think about how ready
 they are.
@@ -282,7 +280,7 @@ master, nor exposed as a part of a stable branch.
 <11> make sure I did not accidentally rewind master beyond what I
 already pushed out.  `ko` shorthand points at the repository I have
 at kernel.org, and looks like this:
-+
+
 
 $ cat .git/remotes/ko
 URL: kernel.org:/pub/scm/git/git.git
@@ -294,7 +292,7 @@ Push: next
 Push: +pu
 Push: maint
 
-+
+
 In the output from `git show-branch`, `master` should have
 everything `ko-master` has, and `next` should have
 everything `ko-next` has.
@@ -322,24 +320,24 @@ example of managing a shared central repository.
 Examples
 
 We assume the following in /etc/services

Re: What's cooking in git.git (Mar 2014, #03; Fri, 14)

2014-03-16 Thread Ramkumar Ramachandra
Philip Oakley wrote:
>> * po/everyday-doc (2014-01-27) 1 commit
>> - Make 'git help everyday' work
>>
>> This may make the said command to emit something, but the source is
>> not meant to be formatted into a manual pages to begin with, and
>> also its contents are a bit stale.  It may be a good first step in
>> the right direction, but needs more work to at least get the
>> mark-up right before public consumption.
>
> I'm not sure what elements you feel need adjustment. At the moment the
> markup formats quite reasonably to my eyes, both as an HTML page and as a
> man page.

I sent you a small patch fixing some minor markup issues.

> That said, the (lack of) introduction could do with a paragraph to introduce
> the "guide". I have something in draft..
>
> I had a thought that it may be worth a slight rearrangement to add a section
> covering the setting up of one's remotes, depending whether it was forked,
> corporate, or independent, but the lack of resolution on the next
> {publish/push} topic makes such a re-write awkward at this stage. (Ram cc'd)

Before attempting to introduce remote.pushdefault and triangular
workflows, I'd first fix the main issue: stale content. I'm not sure
who uses git show-branch or mailx anymore, for instance.
--
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 9/9] sha1_name: add support for @{publish} marks

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> @@ -1068,23 +1069,35 @@ static const char *get_upstream_branch(const char 
> *name_buf, int len)
>  */
> if (!branch)
> die(_("HEAD does not point to a branch"));
> -   if (!branch->merge || !branch->merge[0]->dst) {
> -   if (!ref_exists(branch->refname))
> -   die(_("No such branch: '%s'"), name);
> -   if (!branch->merge) {
> -   die(_("No upstream configured for branch '%s'"),
> -   branch->name);
> +   switch (type) {
> +   case 'u':
> +   if (!branch->merge || !branch->merge[0]->dst) {
> +   if (!ref_exists(branch->refname))
> +   die(_("No such branch: '%s'"), name);
> +   if (!branch->merge) {
> +   die(_("No upstream configured for branch 
> '%s'"),
> +   branch->name);
> +   }
> +   die(
> +   _("Upstream branch '%s' not stored as a 
> remote-tracking branch"),
> +   branch->merge[0]->src);
> +   }
> +   tracking = branch->merge[0]->dst;
> +   break;
> +   case 'p':
> +   if (!branch->push.dst) {
> +   die(_("No publish configured for branch '%s'"),
> +   branch->name);

This assumes a push.default value of 'current' or 'matching'. What
happens if push.default is set to 'nothing' or 'upstream', for
instance?

p.s- Good to see you back on the list :)
--
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 8/9] sha1_name: simplify track finding

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> It's more efficient to check for the braces first.

Why is it more efficient? So you can error out quickly in the case of
a malformed string? I'm personally not thrilled about this change.
--
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 7/9] sha1_name: cleanup interpret_branch_name()

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
>  sha1_name.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)

I like this variable rename. This instance has annoyed me in the past.
--
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 9/9] sha1_name: add support for @{publish} marks

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> diff --git a/sha1_name.c b/sha1_name.c
> index aa3f3e0..a36852d 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -415,9 +415,9 @@ static int ambiguous_path(const char *path, int len)
> return slash;
>  }
>
> -static inline int upstream_mark(const char *string, int len)
> +static inline int tracking_mark(const char *string, int len)
>  {
> -   const char *suffix[] = { "upstream", "u" };
> +   const char *suffix[] = { "upstream", "u", "publish", "p" };

Oh, another thing: on some threads, people decided that "@{push}"
would be a more apt name (+ alias @{u} to @{pull} for symmetry).
--
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 6/9] branch: display publish branch

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras 

Please write a commit message, preferably showing the new git-branch output.

I noticed that this only picks up a publish-branch if
branch.*.pushremote is configured. What happened to the case when
remote.pushdefault is configured?
--
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 v7 03/12] revert/cherry-pick: add --quiet option

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> @@ -635,9 +637,10 @@ static int do_pick_commit(struct commit *commit, struct 
> replay_opts *opts)
> }
>
> if (opts->skip_empty && is_index_unchanged() == 1) {
> -   warning(_("skipping %s... %s"),
> -   find_unique_abbrev(commit->object.sha1, 
> DEFAULT_ABBREV),
> -   msg.subject);
> +   if (!opts->quiet)
> +   warning(_("skipping %s... %s"),
> +   find_unique_abbrev(commit->object.sha1, 
> DEFAULT_ABBREV),
> +   msg.subject);

Personally, I don't see much value in inventing a new option for
suppressing one message.
--
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 v7 04/12] revert/cherry-pick: add --skip option

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> Akin to 'am --skip' and 'rebase --skip'.

I don't recall why my original sequencer series didn't include this
option. Perhaps Jonathan remembers?
--
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 v7 06/12] builtin: add rewrite helper

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> So that we can load and store rewrites, as well as other operations on a
> list of rewritten commits.

Please elaborate. Explain why this code shouldn't go in sequencer.c.
--
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 v7 12/12] cherry-pick: copy notes and run hooks

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> If no action-name is specified, nothing is done.

Why? Is it because git-rebase implements its own notes-copy-on-rewrite logic?

Otherwise, I agree with the goal of making notes.rewrite.
work for cherry-pick.
--
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 v7 12/12] cherry-pick: copy notes and run hooks

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 43631b4..fd085e1 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -248,7 +248,7 @@ pick_one () {
>
> test -d "$rewritten" &&
> pick_one_preserving_merges "$@" && return
> -   output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
> +   output eval git cherry-pick "--action-name ''" "$strategy_args" 
> $empty_args $ff "$@"

Passing an empty action-name looks quite ugly. Is there a better way
to achieve this?
--
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] rebase -i: handle "Nothing to do" case with autostash

2014-05-19 Thread Ramkumar Ramachandra
When a user invokes

  $ git rebase -i @~3

with dirty files and rebase.autostash turned on, and exits the $EDITOR
with an empty buffer, the autostash fails to apply. Although the primary
focus of rr/rebase-autostash was to get the git-rebase--backend.sh
scripts to return control to git-rebase.sh, it missed this case in
git-rebase--interactive.sh. Since this case is unlike the other cases
which return control for housekeeping, assign it a special return status
and handle that return value explicitly in git-rebase.sh.

Reported-by: Karen Etheridge 
Signed-off-by: Ramkumar Ramachandra 
---
 Thanks to Karen for reporting this.

 I chose 2 arbitrarily. Let me know if you have a rationale for other
 return values.

 git-rebase--interactive.sh |  4 ++--
 git-rebase.sh  | 11 ++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..f267d8b 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1049,14 +1049,14 @@ fi
 
 
 has_action "$todo" ||
-   die_abort "Nothing to do"
+   return 2
 
 cp "$todo" "$todo".backup
 git_sequence_editor "$todo" ||
die_abort "Could not execute editor"
 
 has_action "$todo" ||
-   die_abort "Nothing to do"
+   return 2
 
 expand_todo_ids
 
diff --git a/git-rebase.sh b/git-rebase.sh
index 4543815..47ca3b9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -155,7 +155,7 @@ move_to_original_branch () {
esac
 }
 
-finish_rebase () {
+apply_autostash () {
if test -f "$state_dir/autostash"
then
stash_sha1=$(cat "$state_dir/autostash")
@@ -171,6 +171,10 @@ You can run "git stash pop" or "git stash drop" at any 
time.
 '
fi
fi
+}
+
+finish_rebase () {
+   apply_autostash &&
git gc --auto &&
rm -rf "$state_dir"
 }
@@ -186,6 +190,11 @@ run_specific_rebase () {
if test $ret -eq 0
then
finish_rebase
+   elif test $ret -eq 2 # special exit status for rebase -i
+   then
+   apply_autostash &&
+   rm -rf "$state_dir" &&
+   die "Nothing to do"
fi
exit $ret
 }
-- 
2.0.0.rc2.20.gfc2568d.dirty

--
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 2/2] test-config: Add tests for the config_set API

2014-07-06 Thread Ramkumar Ramachandra
A couple of quick nits.

Tanay Abhra wrote:
> +test_expect_success 'clear default config' '
> +   rm -f .git/config
> +'

Unnecessary; a fresh temporary directory is created for each test run.

> +test_expect_success 'initialize default config' '

You might want to mark this as "setup".
--
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/3] for-each-ref: introduce %C(...) for color

2013-11-11 Thread Ramkumar Ramachandra
Junio C Hamano  wrote:
> $ git show -s --format='%CredAnd%CyellowAreNotTheSameColor'

Ouch, this is quite a disaster.

> It would have been much saner if we started from %(color:yellow),
> %(subject), etc., i.e. have a single long-hand magic introducer
> %(...), and added a set of often-used short-hands like %s.
>
> I am not opposed to unify the internal implementations and the
> external interfaces of pretty, for-each-ref and friends, but
> modelling the external UI after the "mnemonic only with ad hoc
> additions" mess the pretty-format uses is a huge mistake.

Okay, I'm convinced; I'll rework the series to do %(color:...) and
resubmit shortly.

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


[PATCH v2 0/3] Minor f-e-r enhacements

2013-11-13 Thread Ramkumar Ramachandra
Hi,

v2 uses the more sane %(color:...) as opposed to %C(...) for changing
the color of the output, as suggested by Junio. Everything else is the
same.

Thanks.

Ramkumar Ramachandra (3):
  for-each-ref: introduce %(HEAD) asterisk marker
  for-each-ref: introduce %(upstream:track[short])
  for-each-ref: introduce %(color:...) for color

 Documentation/git-for-each-ref.txt | 14 +++-
 builtin/for-each-ref.c | 66 ++
 2 files changed, 72 insertions(+), 8 deletions(-)

-- 
1.8.5.rc0.3.g914176d.dirty

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


[PATCH v2 1/3] for-each-ref: introduce %(HEAD) asterisk marker

2013-11-13 Thread Ramkumar Ramachandra
'git branch' shows which branch you are currently on with an '*', but
'git for-each-ref' misses this feature.  So, extend its format with
%(HEAD) for the same effect.

Now you can use the following format in for-each-ref:

  %(HEAD) %(refname:short)

to display an asterisk next to the current ref.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 13 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index f2e08d1..ab3da0e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -93,6 +93,10 @@ upstream::
from the displayed ref. Respects `:short` in the same way as
`refname` above.
 
+HEAD::
+   Used to indicate the currently checked out branch.  Is '*' if
+   HEAD points to the current ref, and ' ' otherwise.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1d4083c..5f1842f 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -75,6 +75,7 @@ static struct {
{ "upstream" },
{ "symref" },
{ "flag" },
+   { "HEAD" },
 };
 
 /*
@@ -675,8 +676,16 @@ static void populate_value(struct refinfo *ref)
v->s = xstrdup(buf + 1);
}
continue;
-   }
-   else
+   } else if (!strcmp(name, "HEAD")) {
+   const char *head;
+   unsigned char sha1[20];
+   head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
+   if (!strcmp(ref->refname, head))
+   v->s = "*";
+   else
+   v->s = " ";
+   continue;
+   } else
continue;
 
formatp = strchr(name, ':');
-- 
1.8.5.rc0.3.g914176d.dirty

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


[PATCH v2 3/3] for-each-ref: introduce %(color:...) for color

2013-11-13 Thread Ramkumar Ramachandra
Enhance 'git for-each-ref' with color formatting options.  You can now
use the following format in for-each-ref:

  %(color:green)%(refname:short)%(color:reset)

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 13 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index c9b192e..2f3ac22 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,6 +101,10 @@ HEAD::
Used to indicate the currently checked out branch.  Is '*' if
HEAD points to the current ref, and ' ' otherwise.
 
+color::
+   Used to change color of output.  Followed by `:`,
+   where names are described in `color.branch.*`.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ed81407..c59fffe 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "parse-options.h"
 #include "remote.h"
+#include "color.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -76,6 +77,7 @@ static struct {
{ "symref" },
{ "flag" },
{ "HEAD" },
+   { "color" },
 };
 
 /*
@@ -662,8 +664,9 @@ static void populate_value(struct refinfo *ref)
!branch->merge[0]->dst)
continue;
refname = branch->merge[0]->dst;
-   }
-   else if (!strcmp(name, "flag")) {
+   } else if (!prefixcmp(name, "color")) {
+   ;
+   } else if (!strcmp(name, "flag")) {
char buf[256], *cp = buf;
if (ref->flag & REF_ISSYMREF)
cp = copy_advance(cp, ",symref");
@@ -729,6 +732,12 @@ static void populate_value(struct refinfo *ref)
else
v->s = "<>";
continue;
+   } else if (!prefixcmp(name, "color")) {
+   char color[COLOR_MAXLEN] = "";
+
+   color_parse(formatp, "--format", color);
+   v->s = xstrdup(color);
+   continue;
} else
die("unknown %.*s format %s",
(int)(formatp - name), name, formatp);
-- 
1.8.5.rc0.3.g914176d.dirty

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


[PATCH v2 2/3] for-each-ref: introduce %(upstream:track[short])

2013-11-13 Thread Ramkumar Ramachandra
Introduce %(upstream:track) to display "[ahead M, behind N]" and
%(upstream:trackshort) to display "=", ">", "<", or "<>"
appropriately (inspired by contrib/completion/git-prompt.sh).

Now you can use the following format in for-each-ref:

  %(refname:short)%(upstream:trackshort)

to display refs with terse tracking information.

Note that :track and :trackshort only work with "upstream", and error
out when used with anything else.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  6 +-
 builtin/for-each-ref.c | 40 +++---
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index ab3da0e..c9b192e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -91,7 +91,11 @@ objectname::
 upstream::
The name of a local ref which can be considered ``upstream''
from the displayed ref. Respects `:short` in the same way as
-   `refname` above.
+   `refname` above.  Additionally respects `:track` to show
+   "[ahead N, behind M]" and `:trackshort` to show the terse
+   version (like the prompt) ">", "<", "<>", or "=".  Has no
+   effect if the ref does not have tracking information
+   associated with it.
 
 HEAD::
Used to indicate the currently checked out branch.  Is '*' if
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 5f1842f..ed81407 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -641,6 +641,7 @@ static void populate_value(struct refinfo *ref)
int deref = 0;
const char *refname;
const char *formatp;
+   struct branch *branch;
 
if (*name == '*') {
deref = 1;
@@ -652,7 +653,6 @@ static void populate_value(struct refinfo *ref)
else if (!prefixcmp(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (!prefixcmp(name, "upstream")) {
-   struct branch *branch;
/* only local branches may have an upstream */
if (prefixcmp(ref->refname, "refs/heads/"))
continue;
@@ -679,6 +679,7 @@ static void populate_value(struct refinfo *ref)
} else if (!strcmp(name, "HEAD")) {
const char *head;
unsigned char sha1[20];
+
head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
if (!strcmp(ref->refname, head))
v->s = "*";
@@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)
continue;
 
formatp = strchr(name, ':');
-   /* look for "short" refname format */
if (formatp) {
+   int num_ours, num_theirs;
+
formatp++;
if (!strcmp(formatp, "short"))
refname = shorten_unambiguous_ref(refname,
  warn_ambiguous_refs);
-   else
+   else if (!strcmp(formatp, "track") &&
+   !prefixcmp(name, "upstream")) {
+   char buf[40];
+
+   stat_tracking_info(branch, &num_ours, 
&num_theirs);
+   if (!num_ours && !num_theirs)
+   v->s = "";
+   else if (!num_ours) {
+   sprintf(buf, "[behind %d]", num_theirs);
+   v->s = xstrdup(buf);
+   } else if (!num_theirs) {
+   sprintf(buf, "[ahead %d]", num_ours);
+   v->s = xstrdup(buf);
+   } else {
+   sprintf(buf, "[ahead %d, behind %d]",
+   num_ours, num_theirs);
+   v->s = xstrdup(buf);
+   }
+   continue;
+   } else if (!strcmp(formatp, "trackshort") &&
+   !prefixcmp(name, "upstream")) {
+
+   stat_tracking_info(branch, &num_ours, 
&num_the

Re: [PATCH] State correct usage of backticks for options in man pages in the coding guidelines

2013-11-13 Thread Ramkumar Ramachandra
Jason St. John wrote:
> + Backticks are used around options or commands:
> +   `--pretty=oneline`
> +   `git rev-list`

You might want to include configuration variables like
`remote.pushdefault` here.
--
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 2/3] for-each-ref: introduce %(upstream:track[short])

2013-11-13 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> builtin/for-each-ref.c: In function 'populate_value':
> builtin/for-each-ref.c:701:13: error: 'refname' may be used uninitialized in 
> this function [-Werror=uninitialized]

In my defense, the gcc on my end (4.8.2) doesn't seem to complain. Are
you using extra cflags?

However, I do get the following compile warnings:

wt-status.c: In function ‘wt_status_print_unmerged_header’:
wt-status.c:187:2: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
  status_printf_ln(s, c, "");
  ^
wt-status.c: In function ‘wt_status_print_cached_header’:
wt-status.c:203:2: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
  status_printf_ln(s, c, "");
  ^
wt-status.c: In function ‘wt_status_print_dirty_header’:
wt-status.c:222:2: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
  status_printf_ln(s, c, "");
  ^
wt-status.c: In function ‘wt_status_print_other_header’:
wt-status.c:234:2: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
  status_printf_ln(s, c, "");
  ^
wt-status.c: In function ‘wt_status_print_trailer’:
wt-status.c:239:2: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
  status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
  ^
wt-status.c: In function ‘wt_status_print_other’:
wt-status.c:767:2: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
  status_printf_ln(s, GIT_COLOR_NORMAL, "");
  ^
wt-status.c: In function ‘wt_status_print_tracking’:
wt-status.c:827:3: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
   fprintf_ln(s->fp, "");
   ^
wt-status.c: In function ‘wt_status_print’:
wt-status.c:1243:3: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
   status_printf(s, color(WT_STATUS_HEADER, s), "");
   ^
wt-status.c:1256:3: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
   status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
   ^
wt-status.c:1258:3: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
   status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
   ^
wt-status.c:1275:4: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
status_printf_ln(s, GIT_COLOR_NORMAL, "");
^
builtin/commit.c: In function ‘prepare_to_commit’:
builtin/commit.c:808:4: warning: zero-length gnu_printf format string
[-Wformat-zero-length]
status_printf_ln(s, GIT_COLOR_NORMAL, "");
^
--
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] for-each-ref: introduce %(color:...) for color

2013-11-13 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> Perhaps like this (obviously not tested as these three patches did
> not add any tests ;-)

Sorry about that. I didn't notice t6300-for-each-ref.sh. Will fix in
the next round.

> I also think that there should be a mechanism to do "color:reset"
> after each record is issued automatically, and also have the color
> output honor --color=auto from the command line, i.e.
>
> git for-each-ref --color=auto --format='%(color:blue)%(subject)' | cat
>
> should turn the coloring off.

We can add --color=auto later, but I'm wondering about auto-reset
color after each token. What happens if I do:

  $ git for-each-ref --format='%(subject)%(color:blue)'

No color, right? So, it should be auto-reset color after each token
_and_ at end of format-string.
--
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 v3 0/6] Replacement for rr/for-each-ref-decoration

2013-11-15 Thread Ramkumar Ramachandra
Hi Junio et al,

v3 contains the following changes:

- [1/6] is totally unrelated; it's a "while we're there" patch.
- [2/6] is required for the test modification in [4/6].
- [3/6], [4/6], [5/6] now come with tests.
- [6/6] is new: implements Junio's suggestion to auto-reset color.

I haven't included Junio's 78465bb (fixup! for-each-ref: introduce
%(upstream:track[short]), 2013-10-31), because my compiler doesn't
complain, and because it's ugly.

Thanks.

Ramkumar Ramachandra (6):
  t6300 (for-each-ref): clearly demarcate setup
  t6300 (for-each-ref): don't hardcode SHA-1 hexes
  for-each-ref: introduce %(HEAD) asterisk marker
  for-each-ref: introduce %(upstream:track[short])
  for-each-ref: introduce %(color:...) for color
  for-each-ref: auto reset color after each atom

 Documentation/git-for-each-ref.txt | 14 ++-
 builtin/for-each-ref.c | 86 +-
 t/t6300-for-each-ref.sh| 59 --
 3 files changed, 135 insertions(+), 24 deletions(-)

-- 
1.8.5.rc0.6.gfd75b41

--
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 v3 4/6] for-each-ref: introduce %(upstream:track[short])

2013-11-15 Thread Ramkumar Ramachandra
Introduce %(upstream:track) to display "[ahead M, behind N]" and
%(upstream:trackshort) to display "=", ">", "<", or "<>"
appropriately (inspired by contrib/completion/git-prompt.sh).

Now you can use the following format in for-each-ref:

  %(refname:short)%(upstream:trackshort)

to display refs with terse tracking information.

Note that :track and :trackshort only work with "upstream", and error
out when used with anything else.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  6 +-
 builtin/for-each-ref.c | 40 +++---
 t/t6300-for-each-ref.sh| 22 +
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index ab3da0e..c9b192e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -91,7 +91,11 @@ objectname::
 upstream::
The name of a local ref which can be considered ``upstream''
from the displayed ref. Respects `:short` in the same way as
-   `refname` above.
+   `refname` above.  Additionally respects `:track` to show
+   "[ahead N, behind M]" and `:trackshort` to show the terse
+   version (like the prompt) ">", "<", "<>", or "=".  Has no
+   effect if the ref does not have tracking information
+   associated with it.
 
 HEAD::
Used to indicate the currently checked out branch.  Is '*' if
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 5f1842f..ed81407 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -641,6 +641,7 @@ static void populate_value(struct refinfo *ref)
int deref = 0;
const char *refname;
const char *formatp;
+   struct branch *branch;
 
if (*name == '*') {
deref = 1;
@@ -652,7 +653,6 @@ static void populate_value(struct refinfo *ref)
else if (!prefixcmp(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (!prefixcmp(name, "upstream")) {
-   struct branch *branch;
/* only local branches may have an upstream */
if (prefixcmp(ref->refname, "refs/heads/"))
continue;
@@ -679,6 +679,7 @@ static void populate_value(struct refinfo *ref)
} else if (!strcmp(name, "HEAD")) {
const char *head;
unsigned char sha1[20];
+
head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
if (!strcmp(ref->refname, head))
v->s = "*";
@@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)
continue;
 
formatp = strchr(name, ':');
-   /* look for "short" refname format */
if (formatp) {
+   int num_ours, num_theirs;
+
formatp++;
if (!strcmp(formatp, "short"))
refname = shorten_unambiguous_ref(refname,
  warn_ambiguous_refs);
-   else
+   else if (!strcmp(formatp, "track") &&
+   !prefixcmp(name, "upstream")) {
+   char buf[40];
+
+   stat_tracking_info(branch, &num_ours, 
&num_theirs);
+   if (!num_ours && !num_theirs)
+   v->s = "";
+   else if (!num_ours) {
+   sprintf(buf, "[behind %d]", num_theirs);
+   v->s = xstrdup(buf);
+   } else if (!num_theirs) {
+   sprintf(buf, "[ahead %d]", num_ours);
+   v->s = xstrdup(buf);
+   } else {
+   sprintf(buf, "[ahead %d, behind %d]",
+   num_ours, num_theirs);
+   v->s = xstrdup(buf);
+   }
+   continue;
+   } else if (!strcmp(formatp, "trackshort") &&
+   !prefixcmp(name, "upstream")) {

[PATCH v3 1/6] t6300 (for-each-ref): clearly demarcate setup

2013-11-15 Thread Ramkumar Ramachandra
Condense the two-step setup into one step, and give it an appropriate
name.

Signed-off-by: Ramkumar Ramachandra 
---
 t/t6300-for-each-ref.sh | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 752f5cb..72d282f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -18,16 +18,13 @@ setdate_and_increment () {
 export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
 }
 
-test_expect_success 'Create sample commit with known timestamp' '
+test_expect_success setup '
setdate_and_increment &&
echo "Using $datestamp" > one &&
git add one &&
git commit -m "Initial" &&
setdate_and_increment &&
git tag -a -m "Tagging at $datestamp" testtag
-'
-
-test_expect_success 'Create upstream config' '
git update-ref refs/remotes/origin/master master &&
git remote add origin nowhere &&
git config branch.master.remote origin &&
-- 
1.8.5.rc0.6.gfd75b41

--
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 v3 3/6] for-each-ref: introduce %(HEAD) asterisk marker

2013-11-15 Thread Ramkumar Ramachandra
'git branch' shows which branch you are currently on with an '*', but
'git for-each-ref' misses this feature.  So, extend its format with
%(HEAD) for the same effect.

Now you can use the following format in for-each-ref:

  %(HEAD) %(refname:short)

to display an asterisk next to the current ref.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 13 +++--
 t/t6300-for-each-ref.sh|  2 ++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index f2e08d1..ab3da0e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -93,6 +93,10 @@ upstream::
from the displayed ref. Respects `:short` in the same way as
`refname` above.
 
+HEAD::
+   Used to indicate the currently checked out branch.  Is '*' if
+   HEAD points to the current ref, and ' ' otherwise.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1d4083c..5f1842f 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -75,6 +75,7 @@ static struct {
{ "upstream" },
{ "symref" },
{ "flag" },
+   { "HEAD" },
 };
 
 /*
@@ -675,8 +676,16 @@ static void populate_value(struct refinfo *ref)
v->s = xstrdup(buf + 1);
}
continue;
-   }
-   else
+   } else if (!strcmp(name, "HEAD")) {
+   const char *head;
+   unsigned char sha1[20];
+   head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
+   if (!strcmp(ref->refname, head))
+   v->s = "*";
+   else
+   v->s = " ";
+   continue;
+   } else
continue;
 
formatp = strchr(name, ':');
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index e1f71ff..5e29ffc 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -77,6 +77,7 @@ test_atom head contents:body ''
 test_atom head contents:signature ''
 test_atom head contents 'Initial
 '
+test_atom head HEAD '*'
 
 test_atom tag refname refs/tags/testtag
 test_atom tag upstream ''
@@ -110,6 +111,7 @@ test_atom tag contents:body ''
 test_atom tag contents:signature ''
 test_atom tag contents 'Tagging at 1151939927
 '
+test_atom tag HEAD ' '
 
 test_expect_success 'Check invalid atoms names are errors' '
test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
-- 
1.8.5.rc0.6.gfd75b41

--
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 v3 2/6] t6300 (for-each-ref): don't hardcode SHA-1 hexes

2013-11-15 Thread Ramkumar Ramachandra
Use rev-parse in its place, making it easier for future patches to
modify the test script.

Signed-off-by: Ramkumar Ramachandra 
---
 t/t6300-for-each-ref.sh | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 72d282f..e1f71ff 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -49,8 +49,8 @@ test_atom head refname refs/heads/master
 test_atom head upstream refs/remotes/origin/master
 test_atom head objecttype commit
 test_atom head objectsize 171
-test_atom head objectname 67a36f10722846e891fbada1ba48ed035de75581
-test_atom head tree 0e51c00fcb93dffc755546f27593d511e1bdb46f
+test_atom head objectname $(git rev-parse refs/heads/master)
+test_atom head tree $(git rev-parse refs/heads/master^{tree})
 test_atom head parent ''
 test_atom head numparent 0
 test_atom head object ''
@@ -82,11 +82,11 @@ test_atom tag refname refs/tags/testtag
 test_atom tag upstream ''
 test_atom tag objecttype tag
 test_atom tag objectsize 154
-test_atom tag objectname 98b46b1d36e5b07909de1b3886224e3e81e87322
+test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag tree ''
 test_atom tag parent ''
 test_atom tag numparent ''
-test_atom tag object '67a36f10722846e891fbada1ba48ed035de75581'
+test_atom tag object $(git rev-parse refs/tags/testtag^0)
 test_atom tag type 'commit'
 test_atom tag author ''
 test_atom tag authorname ''
@@ -302,7 +302,7 @@ test_expect_success 'Check short upstream format' '
 '
 
 cat >expected <expected <<\EOF
-408fe76d02a785a006c2e9c669b7be5589ede96d  
refs/tags/master
-90b5ebede4899eda64893bc2a4c8f1d6fb6dfc40  refs/tags/bogo
+cat >expected < refs/tags/master
+$(git rev-parse refs/tags/bogo)  refs/tags/bogo
 EOF
 
 test_expect_success 'Verify sort with multiple keys' '
-- 
1.8.5.rc0.6.gfd75b41

--
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 v3 5/6] for-each-ref: introduce %(color:...) for color

2013-11-15 Thread Ramkumar Ramachandra
Enhance 'git for-each-ref' with color formatting options.  You can now
use the following format in for-each-ref:

  %(color:green)%(refname:short)%(color:reset)

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 11 +--
 t/t6300-for-each-ref.sh| 14 ++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index c9b192e..2f3ac22 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,6 +101,10 @@ HEAD::
Used to indicate the currently checked out branch.  Is '*' if
HEAD points to the current ref, and ' ' otherwise.
 
+color::
+   Used to change color of output.  Followed by `:`,
+   where names are described in `color.branch.*`.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ed81407..2ff4e54 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "parse-options.h"
 #include "remote.h"
+#include "color.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -76,6 +77,7 @@ static struct {
{ "symref" },
{ "flag" },
{ "HEAD" },
+   { "color" },
 };
 
 /*
@@ -662,8 +664,13 @@ static void populate_value(struct refinfo *ref)
!branch->merge[0]->dst)
continue;
refname = branch->merge[0]->dst;
-   }
-   else if (!strcmp(name, "flag")) {
+   } else if (!prefixcmp(name, "color:")) {
+   char color[COLOR_MAXLEN] = "";
+
+   color_parse(name + 6, "--format", color);
+   v->s = xstrdup(color);
+   continue;
+   } else if (!strcmp(name, "flag")) {
char buf[256], *cp = buf;
if (ref->flag & REF_ISSYMREF)
cp = copy_advance(cp, ",symref");
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 9d874fd..35ca991 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -338,6 +338,20 @@ test_expect_success 'Check for invalid refname format' '
test_must_fail git for-each-ref --format="%(refname:INVALID)"
 '
 
+get_color ()
+{
+   git config --get-color no.such.slot "$1"
+}
+
+cat >expected <actual &&
+   test_cmp expected actual
+'
+
 cat >expected <<\EOF
 heads/master
 tags/master
-- 
1.8.5.rc0.6.gfd75b41

--
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 v3 6/6] for-each-ref: auto reset color after each atom

2013-11-15 Thread Ramkumar Ramachandra
It makes it easier to write a sensible format string, since you don't
have to %(color:reset) after each atom. Additionally, to make sure that
an invocation like the following doesn't leak color,

  $ git for-each-ref --format='%(subject)%(color:green)'

auto-reset at the end of the format string as well.

Signed-off-by: Ramkumar Ramachandra 
---
 builtin/for-each-ref.c  | 22 ++
 t/t6300-for-each-ref.sh |  2 +-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 2ff4e54..1050aea 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -23,6 +23,7 @@ typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 struct atom_value {
const char *s;
unsigned long ul; /* used for sorting when not FIELD_STR */
+   int color : 1;
 };
 
 struct ref_sort {
@@ -669,6 +670,7 @@ static void populate_value(struct refinfo *ref)
 
color_parse(name + 6, "--format", color);
v->s = xstrdup(color);
+   v->color = 1;
continue;
} else if (!strcmp(name, "flag")) {
char buf[256], *cp = buf;
@@ -914,11 +916,9 @@ static void sort_refs(struct ref_sort *sort, struct 
refinfo **refs, int num_refs
qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
 }
 
-static void print_value(struct refinfo *ref, int atom, int quote_style)
+static void print_value(struct atom_value *v, int quote_style)
 {
-   struct atom_value *v;
struct strbuf sb = STRBUF_INIT;
-   get_value(ref, atom, &v);
switch (quote_style) {
case QUOTE_NONE:
fputs(v->s, stdout);
@@ -983,17 +983,31 @@ static void emit(const char *cp, const char *ep)
 static void show_ref(struct refinfo *info, const char *format, int quote_style)
 {
const char *cp, *sp, *ep;
+   struct atom_value *atomv, resetv;
+   int reset_color = 0;
+   char color[COLOR_MAXLEN] = "";
 
+   color_parse("reset", "--format", color);
+   resetv.s = color;
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
ep = strchr(sp, ')');
if (cp < sp)
emit(cp, sp);
-   print_value(info, parse_atom(sp + 2, ep), quote_style);
+   get_value(info, parse_atom(sp + 2, ep), &atomv);
+   print_value(atomv, quote_style);
+   if (reset_color) {
+   print_value(&resetv, quote_style);
+   reset_color = 0;
+   }
+   if (atomv->color)
+   reset_color = 1;
}
if (*cp) {
sp = cp + strlen(cp);
emit(cp, sp);
}
+   if (reset_color)
+   print_value(&resetv, quote_style);
putchar('\n');
 }
 
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 35ca991..2bf2bea 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -348,7 +348,7 @@ $(get_color red)$(git rev-parse --short HEAD)$(get_color 
reset) origin/master
 EOF
 
 test_expect_success 'Check %(color:...) ' '
-   git for-each-ref 
--format="%(color:red)%(objectname:short)%(color:reset) %(upstream:short)" 
refs/heads >actual &&
+   git for-each-ref --format="%(color:red)%(objectname:short) 
%(upstream:short)" refs/heads >actual &&
test_cmp expected actual
 '
 
-- 
1.8.5.rc0.6.gfd75b41

--
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 4/6] for-each-ref: introduce %(upstream:track[short])

2013-11-16 Thread Ramkumar Ramachandra
Eric Sunshine wrote:
> The "prompt" is not mentioned elsewhere in for-each-ref documentation,
> and a person not familiar with contrib/completion/ may be confused by
> this reference. It might make sense instead to explain the meanings of
> ">", "<", "<>", and "=" directly since they are not necessarily
> obvious to the casual reader.

Someone else raised the same point earlier. Okay, I'll elaborate.

>> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
>> index 5f1842f..ed81407 100644
>> --- a/builtin/for-each-ref.c
>> +++ b/builtin/for-each-ref.c
>> @@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)
>> continue;
>>
>> formatp = strchr(name, ':');
>> -   /* look for "short" refname format */
>> if (formatp) {
>> +   int num_ours, num_theirs;
>> +
>> formatp++;
>> if (!strcmp(formatp, "short"))
>> refname = shorten_unambiguous_ref(refname,
>>   warn_ambiguous_refs);
>> -   else
>> +   else if (!strcmp(formatp, "track") &&
>> +   !prefixcmp(name, "upstream")) {
>> +   char buf[40];
>> +
>> +   stat_tracking_info(branch, &num_ours, 
>> &num_theirs);
>> +   if (!num_ours && !num_theirs)
>> +   v->s = "";
>> +   else if (!num_ours) {
>> +   sprintf(buf, "[behind %d]", 
>> num_theirs);
>> +   v->s = xstrdup(buf);
>> +   } else if (!num_theirs) {
>> +   sprintf(buf, "[ahead %d]", num_ours);
>> +   v->s = xstrdup(buf);
>> +   } else {
>> +   sprintf(buf, "[ahead %d, behind %d]",
>
> Is the intention that these strings ("[ahead %d]", etc.) will be
> internationalized in the future? If so, the allocated 40-character
> buffer may be insufficient.

Similar strings in wt-status.c aren't ready for internationalization
either. Besides, these xstrdup() calls leak memory and are quite
suboptimal: if I allocate more memory, I'm going to be leaking more;
so, I'd defer the internationalization discussion until we restructure
this.

>> +   num_ours, num_theirs);
>> +   v->s = xstrdup(buf);
>> +   }
>> +   continue;
>> +   } else if (!strcmp(formatp, "trackshort") &&
>> +   !prefixcmp(name, "upstream")) {
>> +
>> +   stat_tracking_info(branch, &num_ours, 
>> &num_theirs);
>> +   if (!num_ours && !num_theirs)
>> +   v->s = "=";
>> +   else if (!num_ours)
>> +   v->s = "<";
>> +   else if (!num_theirs)
>> +   v->s = ">";
>> +   else
>> +   v->s = "<>";
>> +   continue;
>> +   } else
>> die("unknown %.*s format %s",
>> (int)(formatp - name), name, formatp);
>
> Is it still accurate to call this a "format" in the error message?
> 'track' and 'trackshort' seem more like decorations.

By convention, all f-e-r formatting errors are reported as fatal
errors (see color errors too, for instance), unlike pretty-formats. We
might want to change this in a later series.

>> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
>> index 5e29ffc..9d874fd 100755
>> --- a/t/t6300-for-each-ref.sh
>> +++ b/t/t6300-for-each-ref.sh
>> @@ -303,6 +303,28 @@ test_expect_success 'Check short upstream format' '
>> test_cmp expected actual
>>  '
>>
>> +test_expect_success 'setup for upstream:track[short]' '
>> +   test_commit two
>> +'
>> +
>> +cat >expected <> +[ahead 1]
>> +EOF
>> +
>> +test_expect_success 'Check upstream:track format' '
>> +   git for-each-ref --format="%(upstream:track)" refs/heads >actual &&
>> +   test_cmp expected actual
>> +'
>> +
>> +cat >expected <> +>
>> +EOF
>> +
>> +test_expect_success 'Check upstream:trackshort format' '
>> +   git for-each-ref --format="%(upstream:trackshort)" refs/heads 
>> >actual &&
>> +   test_cmp expected actual
>> +'
>> +
>>  cat >expected <>  $(git rev-parse --short HEAD)
>>  EOF
>
> Would it make sense also to add tests verifying that :track and
> :trackshort correctly fail when applied to a key other

Re: [PATCH v2 3/3] for-each-ref: introduce %(color:...) for color

2013-11-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> If you are saying, by after each token, that
>
> --format='%(color:blue)%(A)literal string%(B)'
>
> should result in
>
>"literal string"  B>
>
> then I would disagree.

Hm, I didn't think it was a bad idea to reset after each token. The
whole point of having color is to make sure that two consecutive
tokens don't have the same color, no? Then again, my scheme would
result in extra unnecessary resets like

  %(color:blue)%(A)%(color:green)%(B)

being turned into:

  %(color:blue)%(A)%(color:reset)%(color:green)%(B)%(color:reset)

Here, the first %(color:reset) is completely unnecessary.

> I was suggesting it to instead produce
>
>   "literal string"   reset>
>
> where the  always comes when some color is used and we
> hit the end of the format string. A bonus point if we can make it so
> that we emit the final reset only when the last "%(color:some)" is
> not "%(color:reset)", but unconditional "reset if we ever used
> color" is fine.

Okay, a simple don't-leak-color. I'll submit another iteration soon.

Thanks.
--
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 v4 0/6] Replacement for rr/for-each-ref-decoration

2013-11-18 Thread Ramkumar Ramachandra
Hi,

The major change since v3 is in [6/6]: color no more auto-resets after
each token, but instead only to avoid leakage (thanks to Junio). Other
minor changes are in accordance with Eric Sunshine's review.

Thanks.

Ramkumar Ramachandra (6):
  t6300 (for-each-ref): clearly demarcate setup
  t6300 (for-each-ref): don't hardcode SHA-1 hexes
  for-each-ref: introduce %(HEAD) asterisk marker
  for-each-ref: introduce %(upstream:track[short])
  for-each-ref: introduce %(color:...) for color
  for-each-ref: avoid color leakage

 Documentation/git-for-each-ref.txt | 14 +-
 builtin/for-each-ref.c | 93 +-
 t/t6300-for-each-ref.sh| 69 ++--
 3 files changed, 151 insertions(+), 25 deletions(-)

-- 
1.8.5.rc0.5.g70ebc73.dirty

--
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 v4 1/6] t6300 (for-each-ref): clearly demarcate setup

2013-11-18 Thread Ramkumar Ramachandra
Condense the two-step setup into one step, and give it an appropriate
name.

Signed-off-by: Ramkumar Ramachandra 
---
 t/t6300-for-each-ref.sh | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 752f5cb..64301e7 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -18,16 +18,13 @@ setdate_and_increment () {
 export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
 }
 
-test_expect_success 'Create sample commit with known timestamp' '
+test_expect_success setup '
setdate_and_increment &&
echo "Using $datestamp" > one &&
git add one &&
git commit -m "Initial" &&
setdate_and_increment &&
-   git tag -a -m "Tagging at $datestamp" testtag
-'
-
-test_expect_success 'Create upstream config' '
+   git tag -a -m "Tagging at $datestamp" testtag &&
git update-ref refs/remotes/origin/master master &&
git remote add origin nowhere &&
git config branch.master.remote origin &&
-- 
1.8.5.rc0.5.g70ebc73.dirty

--
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 v4 6/6] for-each-ref: avoid color leakage

2013-11-18 Thread Ramkumar Ramachandra
To make sure that an invocation like the following doesn't leak color,

  $ git for-each-ref --format='%(subject)%(color:green)'

auto-reset at the end of the format string when the last color token
seen in the format string isn't a color-reset.

Signed-off-by: Ramkumar Ramachandra 
---
 builtin/for-each-ref.c  | 29 +
 t/t6300-for-each-ref.sh |  2 +-
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 2ff4e54..04e35ba 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -23,6 +23,7 @@ typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 struct atom_value {
const char *s;
unsigned long ul; /* used for sorting when not FIELD_STR */
+   int color : 2; /* 1 indicates color, 2 indicates color-reset */
 };
 
 struct ref_sort {
@@ -668,6 +669,10 @@ static void populate_value(struct refinfo *ref)
char color[COLOR_MAXLEN] = "";
 
color_parse(name + 6, "--format", color);
+   if (!strcmp(name + 6, "reset"))
+   v->color = 2;
+   else
+   v->color = 1;
v->s = xstrdup(color);
continue;
} else if (!strcmp(name, "flag")) {
@@ -914,11 +919,9 @@ static void sort_refs(struct ref_sort *sort, struct 
refinfo **refs, int num_refs
qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
 }
 
-static void print_value(struct refinfo *ref, int atom, int quote_style)
+static void print_value(struct atom_value *v, int quote_style)
 {
-   struct atom_value *v;
struct strbuf sb = STRBUF_INIT;
-   get_value(ref, atom, &v);
switch (quote_style) {
case QUOTE_NONE:
fputs(v->s, stdout);
@@ -983,17 +986,35 @@ static void emit(const char *cp, const char *ep)
 static void show_ref(struct refinfo *info, const char *format, int quote_style)
 {
const char *cp, *sp, *ep;
+   struct atom_value *atomv, resetv;
+   int reset_color = 0;
+   char color[COLOR_MAXLEN] = "";
 
+   color_parse("reset", "--format", color);
+   resetv.s = color;
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
ep = strchr(sp, ')');
if (cp < sp)
emit(cp, sp);
-   print_value(info, parse_atom(sp + 2, ep), quote_style);
+   get_value(info, parse_atom(sp + 2, ep), &atomv);
+   print_value(atomv, quote_style);
+
+   /*
+* reset_color is used to avoid color leakage; it
+* should be set when the last color atom is not a
+* color-reset.
+*/
+   if (atomv->color == 1)
+   reset_color = 1;
+   else if (atomv->color == 2)
+   reset_color = 0;
}
if (*cp) {
sp = cp + strlen(cp);
emit(cp, sp);
}
+   if (reset_color)
+   print_value(&resetv, quote_style);
putchar('\n');
 }
 
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 69e3155..46866ba 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -356,7 +356,7 @@ $(git rev-parse --short refs/tags/two) $(get_color 
green)two$(get_color reset)
 EOF
 
 test_expect_success 'Check %(color:...) ' '
-   git for-each-ref --format="%(objectname:short) 
%(color:green)%(refname:short)%(color:reset)" >actual &&
+   git for-each-ref --format="%(objectname:short) 
%(color:green)%(refname:short)" >actual &&
test_cmp expected actual
 '
 
-- 
1.8.5.rc0.5.g70ebc73.dirty

--
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 v4 3/6] for-each-ref: introduce %(HEAD) asterisk marker

2013-11-18 Thread Ramkumar Ramachandra
'git branch' shows which branch you are currently on with an '*', but
'git for-each-ref' misses this feature.  So, extend its format with
%(HEAD) for the same effect.

Now you can use the following format in for-each-ref:

  %(HEAD) %(refname:short)

to display an asterisk next to the current ref.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 13 +++--
 t/t6300-for-each-ref.sh|  2 ++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index f2e08d1..8f87c9a 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -93,6 +93,10 @@ upstream::
from the displayed ref. Respects `:short` in the same way as
`refname` above.
 
+HEAD::
+   '*' if HEAD matches current ref (the checked out branch), ' '
+   otherwise.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1d4083c..5f1842f 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -75,6 +75,7 @@ static struct {
{ "upstream" },
{ "symref" },
{ "flag" },
+   { "HEAD" },
 };
 
 /*
@@ -675,8 +676,16 @@ static void populate_value(struct refinfo *ref)
v->s = xstrdup(buf + 1);
}
continue;
-   }
-   else
+   } else if (!strcmp(name, "HEAD")) {
+   const char *head;
+   unsigned char sha1[20];
+   head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
+   if (!strcmp(ref->refname, head))
+   v->s = "*";
+   else
+   v->s = " ";
+   continue;
+   } else
continue;
 
formatp = strchr(name, ':');
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 675c2a2..1d998f8 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -77,6 +77,7 @@ test_atom head contents:body ''
 test_atom head contents:signature ''
 test_atom head contents 'Initial
 '
+test_atom head HEAD '*'
 
 test_atom tag refname refs/tags/testtag
 test_atom tag upstream ''
@@ -110,6 +111,7 @@ test_atom tag contents:body ''
 test_atom tag contents:signature ''
 test_atom tag contents 'Tagging at 1151939927
 '
+test_atom tag HEAD ' '
 
 test_expect_success 'Check invalid atoms names are errors' '
test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
-- 
1.8.5.rc0.5.g70ebc73.dirty

--
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 v4 2/6] t6300 (for-each-ref): don't hardcode SHA-1 hexes

2013-11-18 Thread Ramkumar Ramachandra
Use rev-parse in its place, making it easier for future patches to
modify the test script.

Signed-off-by: Ramkumar Ramachandra 
---
 t/t6300-for-each-ref.sh | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 64301e7..675c2a2 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -49,8 +49,8 @@ test_atom head refname refs/heads/master
 test_atom head upstream refs/remotes/origin/master
 test_atom head objecttype commit
 test_atom head objectsize 171
-test_atom head objectname 67a36f10722846e891fbada1ba48ed035de75581
-test_atom head tree 0e51c00fcb93dffc755546f27593d511e1bdb46f
+test_atom head objectname $(git rev-parse refs/heads/master)
+test_atom head tree $(git rev-parse refs/heads/master^{tree})
 test_atom head parent ''
 test_atom head numparent 0
 test_atom head object ''
@@ -82,11 +82,11 @@ test_atom tag refname refs/tags/testtag
 test_atom tag upstream ''
 test_atom tag objecttype tag
 test_atom tag objectsize 154
-test_atom tag objectname 98b46b1d36e5b07909de1b3886224e3e81e87322
+test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag tree ''
 test_atom tag parent ''
 test_atom tag numparent ''
-test_atom tag object '67a36f10722846e891fbada1ba48ed035de75581'
+test_atom tag object $(git rev-parse refs/tags/testtag^0)
 test_atom tag type 'commit'
 test_atom tag author ''
 test_atom tag authorname ''
@@ -302,7 +302,7 @@ test_expect_success 'Check short upstream format' '
 '
 
 cat >expected <expected <<\EOF
-408fe76d02a785a006c2e9c669b7be5589ede96d  
refs/tags/master
-90b5ebede4899eda64893bc2a4c8f1d6fb6dfc40  refs/tags/bogo
+cat >expected < refs/tags/master
+$(git rev-parse refs/tags/bogo)  refs/tags/bogo
 EOF
 
 test_expect_success 'Verify sort with multiple keys' '
-- 
1.8.5.rc0.5.g70ebc73.dirty

--
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 v4 5/6] for-each-ref: introduce %(color:...) for color

2013-11-18 Thread Ramkumar Ramachandra
Enhance 'git for-each-ref' with color formatting options.  You can now
use the following format in for-each-ref:

  %(color:green)%(refname:short)%(color:reset)

where color names are described in color.branch.*.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 11 +--
 t/t6300-for-each-ref.sh| 17 +
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 92e82fd..94f5c46 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,6 +101,10 @@ HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
otherwise.
 
+color::
+   Change output color.  Followed by `:`, where names
+   are described in `color.branch.*`.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ed81407..2ff4e54 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "parse-options.h"
 #include "remote.h"
+#include "color.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -76,6 +77,7 @@ static struct {
{ "symref" },
{ "flag" },
{ "HEAD" },
+   { "color" },
 };
 
 /*
@@ -662,8 +664,13 @@ static void populate_value(struct refinfo *ref)
!branch->merge[0]->dst)
continue;
refname = branch->merge[0]->dst;
-   }
-   else if (!strcmp(name, "flag")) {
+   } else if (!prefixcmp(name, "color:")) {
+   char color[COLOR_MAXLEN] = "";
+
+   color_parse(name + 6, "--format", color);
+   v->s = xstrdup(color);
+   continue;
+   } else if (!strcmp(name, "flag")) {
char buf[256], *cp = buf;
if (ref->flag & REF_ISSYMREF)
cp = copy_advance(cp, ",symref");
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index d88d7ac..69e3155 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -343,6 +343,23 @@ test_expect_success 'Check for invalid refname format' '
test_must_fail git for-each-ref --format="%(refname:INVALID)"
 '
 
+get_color ()
+{
+   git config --get-color no.such.slot "$1"
+}
+
+cat >expected <actual &&
+   test_cmp expected actual
+'
+
 cat >expected <<\EOF
 heads/master
 tags/master
-- 
1.8.5.rc0.5.g70ebc73.dirty

--
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 v4 4/6] for-each-ref: introduce %(upstream:track[short])

2013-11-18 Thread Ramkumar Ramachandra
Introduce %(upstream:track) to display "[ahead M, behind N]" and
%(upstream:trackshort) to display "=", ">", "<", or "<>"
appropriately (inspired by contrib/completion/git-prompt.sh).

Now you can use the following format in for-each-ref:

  %(refname:short)%(upstream:trackshort)

to display refs with terse tracking information.

Note that :track and :trackshort only work with "upstream", and error
out when used with anything else.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  6 +-
 builtin/for-each-ref.c | 40 +++---
 t/t6300-for-each-ref.sh| 27 +
 3 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 8f87c9a..92e82fd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -91,7 +91,11 @@ objectname::
 upstream::
The name of a local ref which can be considered ``upstream''
from the displayed ref. Respects `:short` in the same way as
-   `refname` above.
+   `refname` above.  Additionally respects `:track` to show
+   "[ahead N, behind M]" and `:trackshort` to show the terse
+   version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
+   or "=" (in sync).  Has no effect if the ref does not have
+   tracking information associated with it.
 
 HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 5f1842f..ed81407 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -641,6 +641,7 @@ static void populate_value(struct refinfo *ref)
int deref = 0;
const char *refname;
const char *formatp;
+   struct branch *branch;
 
if (*name == '*') {
deref = 1;
@@ -652,7 +653,6 @@ static void populate_value(struct refinfo *ref)
else if (!prefixcmp(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (!prefixcmp(name, "upstream")) {
-   struct branch *branch;
/* only local branches may have an upstream */
if (prefixcmp(ref->refname, "refs/heads/"))
continue;
@@ -679,6 +679,7 @@ static void populate_value(struct refinfo *ref)
} else if (!strcmp(name, "HEAD")) {
const char *head;
unsigned char sha1[20];
+
head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
if (!strcmp(ref->refname, head))
v->s = "*";
@@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)
continue;
 
formatp = strchr(name, ':');
-   /* look for "short" refname format */
if (formatp) {
+   int num_ours, num_theirs;
+
formatp++;
if (!strcmp(formatp, "short"))
refname = shorten_unambiguous_ref(refname,
  warn_ambiguous_refs);
-   else
+   else if (!strcmp(formatp, "track") &&
+   !prefixcmp(name, "upstream")) {
+   char buf[40];
+
+   stat_tracking_info(branch, &num_ours, 
&num_theirs);
+   if (!num_ours && !num_theirs)
+   v->s = "";
+   else if (!num_ours) {
+   sprintf(buf, "[behind %d]", num_theirs);
+   v->s = xstrdup(buf);
+   } else if (!num_theirs) {
+   sprintf(buf, "[ahead %d]", num_ours);
+   v->s = xstrdup(buf);
+   } else {
+   sprintf(buf, "[ahead %d, behind %d]",
+   num_ours, num_theirs);
+   v->s = xstrdup(buf);
+   }
+   continue;
+   } else if (!strcmp(formatp, "trackshort") &&
+ 

Re: [PATCH v4 6/6] for-each-ref: avoid color leakage

2013-11-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
>> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
>> index 2ff4e54..04e35ba 100644
>> --- a/builtin/for-each-ref.c
>> +++ b/builtin/for-each-ref.c
>> @@ -23,6 +23,7 @@ typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } 
>> cmp_type;
>>  struct atom_value {
>>   const char *s;
>>   unsigned long ul; /* used for sorting when not FIELD_STR */
>> + int color : 2; /* 1 indicates color, 2 indicates color-reset */
>>  };
>
> Hmph.  It looks wasteful to have this information in atom_value.

I wanted to avoid an ugly global. On the other end of the spectrum,
modifying the various functions to take an extra reset_color_leakage
parameter seems much too intrusive. Do you have any suggestions?

> Isn't a new single bit in "struct refinfo" all you need to keep
> track of, to see the last %(color:something) you ever saw is for a
> color that is not reset?

No; because I can only look at one atom and set v->color, at a time.
--
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 6/6] for-each-ref: avoid color leakage

2013-11-19 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> My knee-jerk "adding it to struct refinfo" is not correct, either,
> because what we want to know, i.e. "do we need an extra reset?" is
> not a property for each ref.  It is similar to "what is the set of
> atoms the format string is using?" and "do we need to peel tags in
> order to show all information asked by the format string?"
> (i.e. used_atom[] and need_tagged, respectively).

It's mostly cruft carried over from my reset-color-after-each-token
implementation. My severe distaste for global variables prevented me
from looking for the simpler solution.

> Unlike need_tagged which asks "is there any *refname that asks us to
> peel tags?", however, "is the _last_ color:anything in the format
> string not 'reset'?" depends on the order of appearance of atoms in
> the format string, so this needs to be done in a loop that scans the
> format string from left to right once at the very beginning, and we
> have a perfect place to do so in verify_format().

I should be shot for my laziness and lack of ingenuity. Yeah,
verify_format() is a much better place to put the logic than
populate_value().

> So perhaps like this one on top?
>
>  builtin/for-each-ref.c | 45 +
>  1 file changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 04e35ba..5ff51d1 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c

Thanks for taking the time to do this. And apologies for the laziness.
--
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: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
[+CC: Jens, the goto-guy for submodules]

Sergey Sharybin wrote:
> Namely, `git ls-files -m` will show addons as modified, regardless
> ignore=all configuration. In the same time `git diff-index --name-only
> HEAD --` will show no changes at all.

This happens because diff-index handles submodules explicitly (see
diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My
opinion is that this is a bug, and git ls-files needs to be taught to
handle submodules properly.

> This leads to issues with Arcanist (which is a Phabricator's tool) who
> considers addons as uncommited changes and either complains on this or
> just adds this to commits.

Does Arcanist use `git ls-files -m` to check?

> There're also some more issues which happens to our
> developers and which i can not quite reproduce.

Do try to track down the other issues and let us know.

> Sometimes it happens so git checkout to another branch yields about
> uncommited changes to addons and doesn't checkout to another branch.

I've seldom used submodules with branches, so I'll let others chime in.

Cheers.
--
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: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Sergey Sharybin wrote:
> On Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra
>  wrote:
>>
>> [+CC: Jens, the goto-guy for submodules]
>>
>> Sergey Sharybin wrote:
>> > Namely, `git ls-files -m` will show addons as modified, regardless
>> > ignore=all configuration. In the same time `git diff-index --name-only
>> > HEAD --` will show no changes at all.
>>
>> This happens because diff-index handles submodules explicitly (see
>> diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My
>> opinion is that this is a bug, and git ls-files needs to be taught to
>> handle submodules properly.
>
> Shall i fire report somewhere or it's being handled by the folks
> reading this ML?

Bugs are reported and tackled on the list.

>> > This leads to issues with Arcanist (which is a Phabricator's tool) who
>> > considers addons as uncommited changes and either complains on this or
>> > just adds this to commits.
>>
>> Does Arcanist use `git ls-files -m` to check?
>
> Yes, Arcanist uses `git ls-files -m` to check whether there're local
> modifications. We might also contact phab developers asking to change
> it to `git diff --name-only HEAD --`.  Is there a preferable way to
> get list of modified files and are this command intended to output the
> same results?

I just checked it out: it uses `git ls-files -m` to get the list of
unstaged changes; `git diff --name-only HEAD --` will list staged
changes as well.
--
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: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Jeff King wrote:
>> I just checked it out: it uses `git ls-files -m` to get the list of
>> unstaged changes; `git diff --name-only HEAD --` will list staged
>> changes as well.
>
> That diff command compares the working tree and HEAD; if you are trying
> to match `ls-files -m`, you probably wanted just `git diff --name-only`
> to compare the working tree and the index. Although in a script you'd
> probably want to use the plumbing `git diff-files` instead.

Thanks for that. It's probably not worth fixing ls-files; I'll patch
Arcanist to use diff-files instead.
--
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: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Sergey Sharybin wrote:
> Ramkumar, not actually sure what you mean?
>
> For me `git diff --name-only HEAD --` ignores changes to submodules
> hash changes.

`git diff --name-only HEAD --` compares the worktree to HEAD (listing
both staged and unstaged changes); we want `git diff --name-only --`
to compare the worktree to the index (listing only unstaged changes),
as Peff notes.

> Also apparently it became a known TODO for phabricator
> developers [1].

That was me :)

> So, after all is it expected behavior of ls-files or not and if not
> shall i report it as a separate thread? :)

Actually, I doubt it's worth fixing ls-files. Your problem should be
fixed when this is merged (hopefully in a few hours):

  https://github.com/facebook/arcanist/pull/121

Cheers.
--
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: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Sergey Sharybin wrote:
> To reproduce the issue:
>
> - Run git submodule update --recursive to make sure their SHA is
> changed. Then `git add /path/to/changed submodule` or just `git add .`
> - Modify any file from the parent repository
> - Neither of `git status`, `git diff` and `git diff-files --name-only`
> will show changes to a submodule, only changes to that file which was
> changed in parent repo.
> - Make a git commit. It will not list changes to submodule as wll.
> - `git show HEAD` will show changes to both file from and parent
> repository (which is expected) and will also show changes to the
> submodule hash (which is unexpected i'd say).

Thanks Sergey; I can confirm that this is a bug. For some reason, the
`git add .` is adding the ignored submodule to the index. After that,

  $ git diff-index @

is not showing the ignored submodule. Let me see if I can dig through
this in greater detail.
--
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: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Jens Lehmann wrote:
> But the question is if that is the right thing to do: should
> diff.ignoreSubmodules and submodule..ignore only affect
> the diff family or also git log & friends? That would make
> users blind for submodule history (which they already are
> when using diff & friends, so that might be ok here too).

No, I think it's the wrong thing to do. We don't want to show false history.

> The ignore setting is documented to only affect diff output
> (including what checkout, commit and status show as modified).
> While I agree that this behavior is confusing for Sergey and
> not optimal for the floating branch model he uses, git is
> currently doing exactly what it should. And for people using
> the ignore setting to not having to stat submodules with huge
> and/or many files that behavior is what they want: don't bother
> me with what changed, but commit what I did change on purpose.
> We may have to rethink what should happen for users of the
> floating branch model though.

I'd argue that the only reason the diff-family is blind is because the
commit hash changes in the first place; if the hash didn't change (ie.
floating submodules were represented by 0s hash or something), we
wouldn't have this problem. The correct solution is to also make `git
add' blind.
--
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: Re: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Heiko Voigt wrote:
> What I think needs fixing here first is that the ignore setting should not
> apply to any diffs between HEAD and index. IMO, it should only apply
> to the diff between worktree and index.
>
> When we have that the user does not see the submodule changed when
> normally working. But after doing git add . the change to the submodule
> should be shown in status and diff regardless of the configuration.

Yeah, I think this is a good direction.

> After that we can discuss whether add should add submodules that are
> tracked but not shown. How about commit -a ? Should it also ignore the
> change?

Here, I think ignored submodules should behave like files matched by
.gitignore: add should not add (`add -f` would be a good way to force
it), and `commit -a` should also exclude it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: branch annotations?

2013-11-25 Thread Ramkumar Ramachandra
Jeff King wrote:
> I think it makes sense to be able to show it as part of "git branch",
> but the verbose branch listing there is a bit of a mess. Doing it
> cleanly would probably involve refactoring the branch-display code to
> allow users to specify more flexible formats.

Certainly. I'm quite unhappy with the current 'git branch', and am
looking to improve it. Branch descriptions will definitely be useful
to display as well.

> Ramkumar (cc'd) was looking into that refactoring a while back, but I
> did not follow it closely (it looks like some of the underlying
> for-each-ref refactoring is on the 'next' branch?).

The previous effort in collaboration with Duy fell apart,
unfortunately. Currently, there are a few small patches enhancing
f-e-r from the original series in 'next'. Once they graduate, the plan
is to refactor f-e-r a bit so that 'git branch' can slowly be moved
into using its machinery.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] send-email: set SSL options through IO::Socket::SSL::set_client_defaults

2013-12-02 Thread Ramkumar Ramachandra
Thomas Rast wrote:
> When --smtp-encryption=ssl, we use a Net::SMTP::SSL connection,
> passing its ->new all the options that would otherwise go to
> Net::SMTP->new (most options) and IO::Socket::SSL->start_SSL (for the
> SSL options).
>
> However, while Net::SMTP::SSL replaces the underlying socket class
> with an SSL socket, it does nothing to allow passing options to that
> socket.  So the SSL-relevant options are lost.

Both [1/3] and [2/3] look good. However, I'm curious about this one:
Net::SMTP::SSL inherits from IO::Socket::SSL, where new() is defined.
In the documentation for IO::Socket::SSL,

  $ perldoc IO::Socket::SSL

I can see examples where SSL_verify_mode and SSL_ca_path are passed to
new(). So, I'm not sure what this patch is about.
--
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: git-svn troubles with calendarserver SVN repo

2013-12-02 Thread Ramkumar Ramachandra
Matěj Cepl wrote:
> I am trying to git-svn clone
> https://svn.calendarserver.org/repository/calendarserver/CalendarServer/
> and I cannot say I am much succesful.

Consider using (the somewhat experimental) git-remote-testsvn and the
underlying contrib/svn-fe/. For starters, try:

  $ git clone 
testsvn::https://svn.calendarserver.org/repository/calendarserver/CalendarServer/

You might need to hack a bit to get it working properly, but you'll
find that it's much faster than git-svn in the long-run.
--
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/4] completion: prioritize ./git-completion.bash

2013-12-30 Thread Ramkumar Ramachandra
To ease development, prioritize ./git-completion.bash over other
standard system paths.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z "$script" ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e && script="$e" && break
-- 
1.8.5.2.227.g53f3478

--
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/4] Fix branch.autosetup(merge|rebase) completion

2013-12-30 Thread Ramkumar Ramachandra
Hi,

I figured that branch.autosetupmerge, branch.autosetuprebase and
remote.pushdefault are very tedious to type out in full, and started
looking into fixing their completions this evening. The solution turns
out to be much more complex than I initally imagined, but I'm quite
happy with the solution.

I hope it's an enjoyable read.

Ramkumar Ramachandra (4):
  completion: prioritize ./git-completion.bash
  completion: introduce __gitcomp_2 ()
  completion: fix branch.autosetup(merge|rebase)
  completion: fix remote.pushdefault

 contrib/completion/git-completion.bash | 43 --
 contrib/completion/git-completion.zsh  | 12 +-
 2 files changed, 52 insertions(+), 3 deletions(-)

-- 
1.8.5.2.227.g53f3478

--
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/4] completion: introduce __gitcomp_2 ()

2013-12-30 Thread Ramkumar Ramachandra
There are situations where two classes of completions possible. For
example

  branch.

should try to complete

  branch.master.
  branch.autosetupmerge
  branch.autosetuprebase

The first candidate has the suffix ".", and the second/ third candidates
have the suffix " ". To facilitate completions of this kind, create a
variation of __gitcomp_nl () that accepts two sets of arguments and two
independent suffixes.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 30 ++
 contrib/completion/git-completion.zsh  | 10 ++
 2 files changed, 40 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..64b20b8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,36 @@ __gitcomp_nl ()
__gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
 }
 
+# Generates completion reply from two sets of completion words, with
+# configurable suffixes for each.
+#
+# It accepts 2 to 6 arguments:
+# 1: First set of possible completion words.
+# 2: Second set of possible completion words.
+# 3: A prefix to be added to each completion word (both $1 and $2)
+#(optional).
+# 4: Generate possible completion matches for this word (optional).
+# 5: A suffix to be appended to each completion word in the first set
+#($1) instead of the default space (optional).
+# 6: A suffix to be appended to each completion word in the second set
+#($2) instead of the default space (optional).
+__gitcomp_2 ()
+{
+   local pfx="${3-}" cur_="${4-$cur}" sfx="${5- }" sfx2="${6- }" i=0
+   local IFS=$' \t\n'
+
+   for x in $1; do
+   if [[ "$x" == "$cur_"* ]]; then
+   COMPREPLY[i++]="$pfx$x$sfx"
+   fi
+   done
+   for x in $2; do
+   if [[ "$x" == "$cur_"* ]]; then
+   COMPREPLY[i++]="$pfx$x$sfx2"
+   fi
+   done
+}
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 6fca145..261a7f5 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,16 @@ __gitcomp_nl ()
compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 }
 
+__gitcomp_2 ()
+{
+   emulate -L zsh
+
+   local IFS=$' \t\n'
+   compset -P '*[=:]'
+   compadd -Q -S "${5- }" -p "${3-}" -- ${=1} && _ret=0
+   compadd -Q -S "${6- }" -p "${3-}" -- ${=2} && _ret=0
+}
+
 __gitcomp_file ()
 {
emulate -L zsh
-- 
1.8.5.2.227.g53f3478

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


[PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2013-12-30 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config branch.auto

'autosetupmerge' and 'autosetuprebase' don't come up. This is because
"$cur" is matched with "branch.*" and a list of branches are
completed. Add 'autosetup(merge|rebase)' to the list of branches using
__gitcomp_2 ().

Also take care to not complete

  $ git config branch.autosetupmerge.
  $ git config branch.autosetuprebase.

with the usual branch.. candidates.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 64b20b8..0bda757 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1851,12 +1851,18 @@ _git_config ()
;;
branch.*.*)
local pfx="${cur%.*}." cur_="${cur##*.}"
+   if [ "$pfx" == "branch.autosetupmerge." ] ||
+   [ "$pfx" == "branch.autosetuprebase." ]; then
+   return
+   fi
__gitcomp "remote pushremote merge mergeoptions rebase" "$pfx" 
"$cur_"
return
;;
branch.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
-   __gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
+   __gitcomp_2 "$(__git_heads)" "
+   autosetupmerge autosetuprebase
+   " "$pfx" "$cur_" "."
return
;;
guitool.*.*)
-- 
1.8.5.2.227.g53f3478

--
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 4/4] completion: fix remote.pushdefault

2013-12-30 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config remote.push

'pushdefault' doesn't come up. This is because "$cur" is matched with
"remote.*" and a list of remotes are completed. Add 'pushdefault' to the
list of remotes using __gitcomp_2 ().

Also take care to not complete

  $ git config remote.pushdefault.

with the usual remote.. candidates.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 0bda757..9e0213d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1896,6 +1896,9 @@ _git_config ()
;;
remote.*.*)
local pfx="${cur%.*}." cur_="${cur##*.}"
+   if [ "$pfx" == "remote.pushdefault." ]; then
+   return
+   fi
__gitcomp "
url proxy fetch push mirror skipDefaultUpdate
receivepack uploadpack tagopt pushurl
@@ -1904,7 +1907,7 @@ _git_config ()
;;
remote.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
-   __gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
+   __gitcomp_2 "$(__git_remotes)" "pushdefault" "$pfx" "$cur_" "."
return
;;
url.*.*)
-- 
1.8.5.2.227.g53f3478

--
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] for-each-ref: remove unused variable

2013-12-30 Thread Ramkumar Ramachandra
Signed-off-by: Ramkumar Ramachandra 
---
 builtin/for-each-ref.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 6551e7b..51798b4 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -92,7 +92,7 @@ static struct {
  */
 static const char **used_atom;
 static cmp_type *used_atom_type;
-static int used_atom_cnt, sort_atom_limit, need_tagged, need_symref;
+static int used_atom_cnt, need_tagged, need_symref;
 static int need_color_reset_at_eol;
 
 /*
@@ -1105,7 +1105,6 @@ int cmd_for_each_ref(int argc, const char **argv, const 
char *prefix)
 
if (!sort)
sort = default_sort();
-   sort_atom_limit = used_atom_cnt;
 
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
-- 
1.8.5.2.227.g53f3478

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


Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-02 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> If we are looking at "branch.autosetupmerge." followed by something,
> who typed that final dot?

I admit that it's a very unlikely case. The user did:

  $ branch.autosetupmer

hit backspace to delete the trailing space, inserted a dot, and hit  again.

> If you are working on a topic about
> auto-setup-merge and named your branch "autosetupmerge", don't you
> want to be able to configure various aspect of that branch via
> branch.autosetupmerge.{remote,merge} etc., just like you can do so
> for your "topic" branch via branch.topic.{remote,merge} etc.,
> regardless of your use of "autosetupmerge" option across branches?

My reasoning was that being correct was more important that being
complete. So, if by some horrible chance, the user names her branch
"autosetupmerge", we don't aid her in completions.

> Besides, it smells fishy to me that you need to enumerate and
> special case these two here, and then you have to repeat them below
> in a separate case arm.

I'm not too irked about correctness in this odd case; seeing that you
aren't either, I'll resubmit the series without this hunk (+ the hunk
in remote.pushdefault).
--
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 2/4] completion: introduce __gitcomp_2 ()

2014-01-02 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> __gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
> __gitcomp_nl_append $"autosetupmerge\nautosetuprebase\n" "$pfx" 
> "$cur_" " "

This is not a bad idea at all. I'm just afraid that we might be
leaving open ends: What happens if the $pfx isn't the same in both
cases? Who keeps track of the index "i" of COMPREPLY (it's currently a
local variable)? If we make it global, doesn't every function that
deals with COMPREPLY be careful to reset it?

More importantly, can you see a usecase for more than two completion classes?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/4] completion: prioritize ./git-completion.bash

2014-01-03 Thread Ramkumar Ramachandra
To ease development, prioritize ./git-completion.bash over other
standard system paths.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z "$script" ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e && script="$e" && break
-- 
1.8.5.2.227.g53f3478

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


[PATCH v2 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-03 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config branch.auto

'autosetupmerge' and 'autosetuprebase' don't come up. This is because
"$cur" is matched with "branch.*" and a list of branches are
completed. Add 'autosetup(merge|rebase)' to the list of branches using
__gitcomp_2 ().

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 64b20b8..cbb4eca 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1856,7 +1856,9 @@ _git_config ()
;;
branch.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
-   __gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
+   __gitcomp_2 "$(__git_heads)" "
+   autosetupmerge autosetuprebase
+   " "$pfx" "$cur_" "."
return
;;
guitool.*.*)
-- 
1.8.5.2.227.g53f3478

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


[PATCH v2 4/4] completion: fix remote.pushdefault

2014-01-03 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config remote.push

'pushdefault' doesn't come up. This is because "$cur" is matched with
"remote.*" and a list of remotes are completed. Add 'pushdefault' to the
list of remotes using __gitcomp_2 ().

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index cbb4eca..04d72a1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1900,7 +1900,7 @@ _git_config ()
;;
remote.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
-   __gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
+   __gitcomp_2 "$(__git_remotes)" "pushdefault" "$pfx" "$cur_" "."
return
;;
url.*.*)
-- 
1.8.5.2.227.g53f3478

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


[PATCH v2 2/4] completion: introduce __gitcomp_2 ()

2014-01-03 Thread Ramkumar Ramachandra
There are situations where two classes of completions possible. For
example

  branch.

should try to complete

  branch.master.
  branch.autosetupmerge
  branch.autosetuprebase

The first candidate has the suffix ".", and the second/ third candidates
have the suffix " ". To facilitate completions of this kind, create a
variation of __gitcomp_nl () that accepts two sets of arguments and two
independent suffixes.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 30 ++
 contrib/completion/git-completion.zsh  | 10 ++
 2 files changed, 40 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..64b20b8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,36 @@ __gitcomp_nl ()
__gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
 }
 
+# Generates completion reply from two sets of completion words, with
+# configurable suffixes for each.
+#
+# It accepts 2 to 6 arguments:
+# 1: First set of possible completion words.
+# 2: Second set of possible completion words.
+# 3: A prefix to be added to each completion word (both $1 and $2)
+#(optional).
+# 4: Generate possible completion matches for this word (optional).
+# 5: A suffix to be appended to each completion word in the first set
+#($1) instead of the default space (optional).
+# 6: A suffix to be appended to each completion word in the second set
+#($2) instead of the default space (optional).
+__gitcomp_2 ()
+{
+   local pfx="${3-}" cur_="${4-$cur}" sfx="${5- }" sfx2="${6- }" i=0
+   local IFS=$' \t\n'
+
+   for x in $1; do
+   if [[ "$x" == "$cur_"* ]]; then
+   COMPREPLY[i++]="$pfx$x$sfx"
+   fi
+   done
+   for x in $2; do
+   if [[ "$x" == "$cur_"* ]]; then
+   COMPREPLY[i++]="$pfx$x$sfx2"
+   fi
+   done
+}
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 6fca145..261a7f5 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,16 @@ __gitcomp_nl ()
compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 }
 
+__gitcomp_2 ()
+{
+   emulate -L zsh
+
+   local IFS=$' \t\n'
+   compset -P '*[=:]'
+   compadd -Q -S "${5- }" -p "${3-}" -- ${=1} && _ret=0
+   compadd -Q -S "${6- }" -p "${3-}" -- ${=2} && _ret=0
+}
+
 __gitcomp_file ()
 {
emulate -L zsh
-- 
1.8.5.2.227.g53f3478

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


[PATCH v2 0/4] Fix branch.autosetup(merge|rebase) completion

2014-01-03 Thread Ramkumar Ramachandra
Hi,

In this iteration, I've removed hunks to prevent completing:

  $ git config remote.pushdefault.
  $ git config branch.autosetupmerge.
  $ git config branch.autosetuprebase.

Since they're perfectly valid remote/ branch names.

Thanks.

Ramkumar Ramachandra (4):
  completion: prioritize ./git-completion.bash
  completion: introduce __gitcomp_2 ()
  completion: fix branch.autosetup(merge|rebase)
  completion: fix remote.pushdefault

 contrib/completion/git-completion.bash | 36 --
 contrib/completion/git-completion.zsh  | 12 +++-
 2 files changed, 45 insertions(+), 3 deletions(-)

-- 
1.8.5.2.227.g53f3478

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


Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-03 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> You seem to be calling it "incorrect" to give the same degree of
> completion for a branch the user named "autosetupmerge" as another
> branch "topic", but I think it is incorrect not to, so I cannot tell
> if we are agreeing or disagreeing.

No, what's incorrect is providing completions for

  $ git config branch.autosetupmerge.

when no branch called "autosetupmerge" exists. The purpose of the hunk
(which I now removed) was to prevent such completions, but it has the
side-effect of also preventing a legitimate completion in the case
when the user really has a branch named "autosetupmerge".

What is your take on the issue?
--
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 2/4] completion: introduce __gitcomp_2 ()

2014-01-03 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> I am not sure what you are worried about $pfx; what does it do when
> you have strings with different prefix in COMPREPLY? If it breaks,
> then the answer is "don't do it then".
>
> Doesn't an array know its own length and give you a way to ask?

Right. I was just throwing counterpoints at the wall.

> Imagine if one is flipping 47 topic branches from 6 contributors
> whose names all begin with 'j'.  I can see that such a person would
> appreciate if "git config branch.j" did not dump all 47 topics
> at once but offered "jc/ jk/ jl/ jm/ jn/ js/" instead, and then a
> follow-up completion of "git config branch.jk/" expanded to
> names of topics from that single contributor "jk".  Wouldn't the way
> to give these be either to return these two-letter hierarchy names
> with slash as the suffix or to return list of two-letter plus a
> slash with an empty suffix?  Either way, that is using something
> different from a dot or a space, so that may count as the third, I
> guess.

Ah, after completing branch.jk/, we would want no suffix: so the
example definitely counts as a third "completion class". I agree with
your reasoning, and will rework the series to do the _append () thing
you suggested.

Thanks.
--
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 v3 0/4] Fix branch.autosetup(merge|rebase) completion

2014-01-03 Thread Ramkumar Ramachandra
Hi Junio et al,

In v3, I've implemented __gitcomp_nl_append (), like you
suggested. After implementing it, I feel foolish about having gone
around my head to do __gitcomp_2 ().

Thanks.

Ramkumar Ramachandra (4):
  completion: prioritize ./git-completion.bash
  completion: introduce __gitcomp_nl_append ()
  completion: fix branch.autosetup(merge|rebase)
  completion: fix remote.pushdefault

 contrib/completion/git-completion.bash | 15 +++
 contrib/completion/git-completion.zsh  | 10 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

-- 
1.8.5.2.227.g53f3478

--
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 v3 1/4] completion: prioritize ./git-completion.bash

2014-01-03 Thread Ramkumar Ramachandra
To ease development, prioritize ./git-completion.bash over other
standard system paths.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z "$script" ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e && script="$e" && break
-- 
1.8.5.2.227.g53f3478

--
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 v3 2/4] completion: introduce __gitcomp_nl_append ()

2014-01-03 Thread Ramkumar Ramachandra
There are situations where multiple classes of completions possible. For
example

  branch.

should try to complete

  branch.master.
  branch.autosetupmerge
  branch.autosetuprebase

The first candidate has the suffix ".", and the second/ third candidates
have the suffix " ". To facilitate completions of this kind, create a
variation of __gitcomp_nl () that appends to the existing list of
completion candidates, COMPREPLY.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 13 +
 contrib/completion/git-completion.zsh  |  8 
 2 files changed, 21 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..bf358d6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,19 @@ __gitcomp_nl ()
__gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
 }
 
+# Variation of __gitcomp_nl () that appends to the existing list of
+# completion candidates, COMPREPLY.
+__gitcomp_nl_append ()
+{
+   local IFS=$'\n'
+   local i=${#COMPREPLY[@]}
+   for x in $1; do
+   if [[ "$x" == "$3"* ]]; then
+   COMPREPLY[i++]="$2$x$4"
+   fi
+   done
+}
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 6fca145..6b77968 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,14 @@ __gitcomp_nl ()
compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 }
 
+__gitcomp_nl_append ()
+{
+   emulate -L zsh
+
+   local IFS=$'\n'
+   compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+}
+
 __gitcomp_file ()
 {
emulate -L zsh
-- 
1.8.5.2.227.g53f3478

--
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 v3 4/4] completion: fix remote.pushdefault

2014-01-03 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config remote.push

'pushdefault' doesn't come up. This is because "$cur" is matched with
"remote.*" and a list of remotes are completed. Add 'pushdefault' to the
list of remotes using __gitcomp_nl_append ().

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 75c7302..345ceff 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1883,6 +1883,7 @@ _git_config ()
remote.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
+   __gitcomp_nl_append "pushdefault" "$pfx" "$cur_"
return
;;
url.*.*)
-- 
1.8.5.2.227.g53f3478

--
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 v3 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-03 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config branch.auto

'autosetupmerge' and 'autosetuprebase' don't come up. This is because
"$cur" is matched with "branch.*" and a list of branches are
completed. Add 'autosetupmerge', 'autosetuprebase' to the list of
branches using __gitcomp_nl_append ().

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index bf358d6..75c7302 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1840,6 +1840,7 @@ _git_config ()
branch.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
__gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
+   __gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' "$pfx" 
"$cur_"
return
;;
guitool.*.*)
-- 
1.8.5.2.227.g53f3478

--
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 2/4] completion: introduce __gitcomp_nl_append ()

2014-01-05 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> Is it because going this route and doing it at such a low level
> would make zsh completion (which I have no clue about ;-)
> unnecessarily complex?

The zsh completion only cares to override __gitcomp_nl () and
__gitcomp_nl_append (), without bothering to re-implement the
lower-level functions; so it's no problem at all. I wrote mine out by
thinking of a non-intrusive direct translation, while your version
focuses on eliminating duplication. I don't have a strong preference
either way, so I will resubmit with your version.
--
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/4] completion: prioritize ./git-completion.bash

2014-01-05 Thread Ramkumar Ramachandra
brian m. carlson wrote:
> I'm not clear on this change.  It looks like this loads
> git-completion.bash from the same directory as git-completion.zsh.  Is
> this correct?

Yes, and that's what I meant to convey with the "./". Junio's message
is clearer though, so I'll use that.

Thanks.
--
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 v4 0/4] Re-spin rr/completion-branch-config

2014-01-05 Thread Ramkumar Ramachandra
Hi Junio et al,

Most significantly, [2/4] no longer duplicates __gitcompadd () in
__gitcomp_nl_append (). Other than that, the commit messages for the
other patches are improved.

Thanks.

Ramkumar Ramachandra (4):
  zsh completion: find matching custom bash completion
  completion: introduce __gitcomp_nl_append ()
  completion: fix branch.autosetup(merge|rebase)
  completion: fix remote.pushdefault

 contrib/completion/git-completion.bash | 24 
 contrib/completion/git-completion.zsh  | 10 +-
 2 files changed, 29 insertions(+), 5 deletions(-)

-- 
1.8.5.2.227.g53f3478

--
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 v4 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-05 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config branch.auto

'autosetupmerge' and 'autosetuprebase' don't come up. This is because
"$cur" is matched with "branch.*" and a list of branches are
completed. Add 'autosetupmerge', 'autosetuprebase' as candidates for
completion too, using __gitcomp_nl_append ().

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 20febff..a57bcbe 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1841,6 +1841,7 @@ _git_config ()
branch.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
__gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
+   __gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' "$pfx" 
"$cur_"
return
;;
guitool.*.*)
-- 
1.8.5.2.227.g53f3478

--
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 v4 4/4] completion: fix remote.pushdefault

2014-01-05 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config remote.push

'pushdefault' doesn't come up. This is because "$cur" is matched with
"remote.*" and a list of remotes are completed. Add 'pushdefault' as a
candidate for completion too, using __gitcomp_nl_append ().

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index a57bcbe..4fe5ce3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1884,6 +1884,7 @@ _git_config ()
remote.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
+   __gitcomp_nl_append "pushdefault" "$pfx" "$cur_"
return
;;
url.*.*)
-- 
1.8.5.2.227.g53f3478

--
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 v4 2/4] completion: introduce __gitcomp_nl_append ()

2014-01-05 Thread Ramkumar Ramachandra
There are situations where multiple classes of completions possible. For
example

  branch.

should try to complete

  branch.master.
  branch.autosetupmerge
  branch.autosetuprebase

The first candidate has the suffix ".", and the second/ third candidates
have the suffix " ". To facilitate completions of this kind, create a
variation of __gitcomp_nl () that appends to the existing list of
completion candidates, COMPREPLY.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 22 ++
 contrib/completion/git-completion.zsh  |  8 
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..20febff 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -178,9 +178,9 @@ _get_comp_words_by_ref ()
 }
 fi
 
-__gitcompadd ()
+__gitcompappend ()
 {
-   local i=0
+   local i=${#COMPREPLY[@]}
for x in $1; do
if [[ "$x" == "$3"* ]]; then
COMPREPLY[i++]="$2$x$4"
@@ -188,6 +188,12 @@ __gitcompadd ()
done
 }
 
+__gitcompadd ()
+{
+   COMPREPLY=()
+   __gitcompappend "$@"
+}
+
 # Generates completion reply, appending a space to possible completion words,
 # if necessary.
 # It accepts 1 to 4 arguments:
@@ -218,6 +224,14 @@ __gitcomp ()
esac
 }
 
+# Variation of __gitcomp_nl () that appends to the existing list of
+# completion candidates, COMPREPLY.
+__gitcomp_nl_append ()
+{
+   local IFS=$'\n'
+   __gitcompappend "$1" "${2-}" "${3-$cur}" "${4- }"
+}
+
 # Generates completion reply from newline-separated possible completion words
 # by appending a space to all of them.
 # It accepts 1 to 4 arguments:
@@ -229,8 +243,8 @@ __gitcomp ()
 #appended.
 __gitcomp_nl ()
 {
-   local IFS=$'\n'
-   __gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
+   COMPREPLY=()
+   __gitcomp_nl_append "$@"
 }
 
 # Generates completion reply with compgen from newline-separated possible
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 6fca145..6b77968 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,14 @@ __gitcomp_nl ()
compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 }
 
+__gitcomp_nl_append ()
+{
+   emulate -L zsh
+
+   local IFS=$'\n'
+   compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+}
+
 __gitcomp_file ()
 {
emulate -L zsh
-- 
1.8.5.2.227.g53f3478

--
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 v4 1/4] zsh completion: find matching custom bash completion

2014-01-05 Thread Ramkumar Ramachandra
If zsh completion is being read from a location that is different from
system-wide default, it is likely that the user is trying to use a
custom version, perhaps closer to the bleeding edge, installed in her
own directory. We will more likely to find the matching bash completion
script in the same directory than in those system default places.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z "$script" ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e && script="$e" && break
-- 
1.8.5.2.227.g53f3478

--
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] Minor convinience feature: format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Hi,

This is inspired by merge.defaultToUpstream. Disclaimer: I have an
"fpm" alias for doing this (separate from "fp" for when I need to
generate patches against some other branch), which I'd like to get rid
of.

Thanks.

Ramkumar Ramachandra (2):
  completion: complete format.coverLetter
  format-patch: introduce format.defaultTo

 Documentation/config.txt   |  4 
 builtin/log.c  |  7 +++
 contrib/completion/git-completion.bash |  2 ++
 t/t4014-format-patch.sh| 10 ++
 4 files changed, 23 insertions(+)

-- 
1.8.5.2.229.g4448466.dirty

--
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] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
A very common workflow for preparing patches involves working off a
topic branch and generating patches against 'master' to send off to the
maintainer. However, a plain

  $ git format-patch -o outgoing

is a no-op on a topic branch, and the user has to remember to specify
'master' explicitly everytime. Save the user the extra keystrokes by
introducing format.defaultTo which can contain the name of a branch
against which to base patches.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/config.txt   |  4 
 builtin/log.c  |  7 +++
 contrib/completion/git-completion.bash |  1 +
 t/t4014-format-patch.sh| 10 ++
 4 files changed, 22 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..b90abd1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1135,6 +1135,10 @@ format.coverLetter::
format-patch is invoked, but in addition can be set to "auto", to
generate a cover-letter only when there's more than one patch.
 
+format.defaultTo::
+   The name of a branch against which to generate patches by
+   default. You'd usually want this to be 'master'.
+
 filter..clean::
The command which is used to convert the content of a worktree
file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/builtin/log.c b/builtin/log.c
index b97373d..ebc419e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -674,6 +674,7 @@ static int thread;
 static int do_signoff;
 static const char *signature = git_version_string;
 static int config_cover_letter;
+static const char *config_defaultto;
 
 enum {
COVER_UNSET,
@@ -750,6 +751,8 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON : 
COVER_OFF;
return 0;
}
+   if (!strcmp(var, "format.defaultto"))
+   return git_config_string(&config_defaultto, var, value);
 
return git_log_config(var, value, cb);
 }
@@ -1324,6 +1327,10 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
die (_("--subject-prefix and -k are mutually exclusive."));
rev.preserve_subject = keep_subject;
 
+   if (argc < 2 && config_defaultto) {
+   argv[1] = config_defaultto;
+   argc++;
+   }
argc = setup_revisions(argc, argv, &rev, &s_r_opt);
if (argc > 1)
die (_("unrecognized argument: %s"), argv[1]);
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 39b81f7..75699d4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1992,6 +1992,7 @@ _git_config ()
format.attach
format.cc
format.coverLetter
+   format.defaultTo
format.headers
format.numbered
format.pretty
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 73194b2..46c0337 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1370,4 +1370,14 @@ test_expect_success 'cover letter auto user override' '
test_line_count = 2 list
 '
 
+test_expect_success 'defaultTo side' '
+   mkdir -p tmp &&
+   test_when_finished "rm -rf tmp;
+   git config --unset format.defaultTo" &&
+
+   git config format.defaultTo side &&
+   git format-patch -o tmp >list &&
+   test_line_count = 3 list
+'
+
 test_done
-- 
1.8.5.2.229.g4448466.dirty

--
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] completion: complete format.coverLetter

2014-01-06 Thread Ramkumar Ramachandra
Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..39b81f7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1991,6 +1991,7 @@ _git_config ()
fetch.unpackLimit
format.attach
format.cc
+   format.coverLetter
format.headers
format.numbered
format.pretty
-- 
1.8.5.2.229.g4448466.dirty

--
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] Minor convinience feature: format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
> Ramkumar Ramachandra (2):
>   completion: complete format.coverLetter
>   format-patch: introduce format.defaultTo

Any thoughts on checkout.defaultTo? I have a "com" alias to checkout 'master'.
--
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] rebase --onto doesn't abort properly

2014-01-06 Thread Ramkumar Ramachandra
Hi,

On the latest git, I noticed that a rebase --onto doesn't abort
properly. Steps to reproduce:

  # on some topic branch
  $ git rebase --onto master @~10
  ^C # quickly!
  $ git rebase --abort
  # HEAD is still detached

I tried going back a few revisions, and the bug seems to be very old;
I'm surprised I didn't notice it earlier.

Are others able to reproduce this?

Thanks.

Ram
--
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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
>  - why is a single branch name sufficient?

It does accept a , so any form is allowed; but why would
anyone want that in a format.defaultTo? I'm not sure we want to impose
an artificial restriction on the configuration variable though.

>  - is it a better option to simply default to @{u}, if one exists,
>instead of failing?

I'm not sure @{u} is a good default. Personally, my workflow involves
publishing my fork before sending out patches; mainly so that I can
compare with @{u} when I do re-spins. People can put @{u} in
format.defaultTo if it suits their workflow though.
--
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: [BUG] rebase --onto doesn't abort properly

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> I do not think --abort was designed to abort an uncontrolled stop
> like ^C in the first place.

Why not? All it requires is a reset --hard to
.git/rebase-apply/head-name, as usual, no?

> To allow that kind of "recovery", you
> need to teach "rebase" to first record the state you would want to
> go back to before doing anything, but then what happens if the ^C
> comes when you are still in the middle of doing so?

I'm a bit puzzled: doesn't rebase write_basic_state() as soon as it
starts? It's true that we can't recover from a ^C before that, but I
would expect to be able to recover after a ^C somewhere in the middle.
--
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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Jonathan Nieder wrote:
>  1. Most config settings are in noun form: e.g.,
> "[remote] pushDefault = foo".  That makes their names easy to guess
> and makes them easy to talk about: I set the default remote for
> pushing by changing the remote.pushdefault setting.
>
> '[url ""] insteadOf' is an exception to that and a bit of an
> aberration.
>
> This new '[format] defaultTo' repeats the same end-with-a-preposition
> mistake, while I think it would be better to learn from it.

I agree that it's somewhat unconventional to allow people to put a
 as a configuration variable value, but I think it's useful.
url..insteadOf is incredibly useful, for instance.

>  2. Wouldn't a more natural default be @{u}..HEAD instead of relying on
> the user to do the make-work of keeping a local branch that tracks
> master up to date?

Like I said in my message to Junio, @{u} is not necessarily the "best"
default for all workflows, although you can fill that into
format.defaultTo.
--
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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> I meant "a single branch" as opposed to "depending on what branch
> you are sending out, you may have to use a different upstream
> starting point", and a single "format.defaultTo" that does not read
> what your HEAD currently points at may not be enough.
>
> Unless you set @{u} to this new configuration, in which case the
> choice becomes dynamic depending on the current branch, but
>
>  - if that is the only sane choice based on the current branch, why
>not use that as the default without having to set the
>configuration?
>
>  - Or if that is still insufficient, don't we need branch.*.forkedFrom
>that is different from branch.*.merge, so that different branches
>you want to show "format-patch" output can have different
>reference points?

Ah, I was going for an equivalent of merge.defaultToUpstream, but I
guess branch.*.forkedFrom is a good way to go.
--
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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Jeff King wrote:
> Yeah, I had similar thoughts. I personally use "branch.*.merge" as
> "forkedFrom", and it seems like we are going that way anyway with things
> like "git rebase" and "git merge" defaulting to upstream.

My issue with that is that I no idea where my branch is with respect
to my forked upstream; I find that extremely useful when doing
re-spins.

> But then there
> is "git push -u" and "push.default = upstream", which treats the
> upstream config as something else entirely.

push.default = upstream is a bit of a disaster, in my opinion. I've
advocated push.default = current on multiple occasions, and wrote the
initial remote.pushDefault series with that configuration in mind.

> I wonder if it is too late to try to clarify this dual usage. It kind of
> seems like the push config is "this is the place I publish to". Which,
> in many workflows, just so happens to be the exact same as the place you
> forked from. Could we introduce a new branch.*.pushupstream variable
> that falls back to branch.*.merge? Or is that just throwing more fuel on
> the fire (more sand in the pit in my analogy, I guess).

We already have a branch.*.pushremote, and I don't see the value of
branch.*.pushbranch (what you're referring to as pushupstream, I
assume) except for Gerrit users. Frankly, I don't use full triangular
workflows myself mainly because my prompt is compromised: when I have
a branch.*.remote different from branch.*.pushremote, I'd like to see
where my branch is with respect to @{u} and @{publish} (not yet
invented); that's probably too much information to digest anyway, so I
use central workflow (pointing to my fork) for each of my branches,
except master (which points to Junio's repo).

> I admit I haven't thought it through yet, though. And even if it does
> work, it may throw a slight monkey wrench in the proposed push.default
> transition.

We're transitioning to push.default = simple which is even simpler than current.
--
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 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:.
> As I said in the different subthread, I am not convinced that you
> would need the complexity of branch.*.forkedFrom.  If you set your
> "upstream" to the true upstream (not your publishing point), and
> have "remote.pushdefault"set to 'publish', you can expect
>
> git push
>
> to do the right thing, and then always say
>
> git show-branch publish/topic topic

I think it's highly sub-optimal to have a local-branch @{u} for
several reasons; the prompt is almost useless in this case, and it
will always show your forked-branch ahead of 'master' (assuming that
'master' doesn't update itself in the duration of your development).
While doing respins, the prompt doesn't aid you in any way. Besides,
on several occasions, I found myself working on the same forked-branch
from two different machines; then the publish-point isn't necessarily
always a publish-point: it's just another "upstream" for the branch.
The point of a special branch.*.forkedFrom is that you can always show
the "master..@" information in the prompt ignoring divergences; after
all, a divergence simply means that you need to rebase onto the latest
'master' ('master' is never going to get a non-ff update anyway).

So, instead of crippling the functionality around the publish-point,
let's build minimal required functionality around branch.*.forkedFrom.
I'd love a prompt like:

  branch-forkedfrom<>5*:~/src/git$

Clearly, branch-forkedfrom has diverged from my publish-point (I'm
probably doing a respin), and has 5 commits (it's 5 commits ahead of
'master' ignoring divergences).

> to see where your last published branch is relative to what you
> have, no?  Mapping "topic@{publish}" to "refs/remotes/publish/topic"
> (or when you have 'topic' checked out, mapping "@{publish}" to it)
> is a trivial but is a separate usefulness, I would guess.

I think a @{publish} is needed for when branch.*.remote is different
from remote.pushDefault. Like I said earlier, it's probably too much
information to give to the user: divergences with respect to two
remotes. So, we'll hold it off until someone finds a usecase for it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
John Szakmeister wrote:
> Then where does it get pushed?  Do you always specify where to save your work?
>
> FWIW, I think the idea of treating @{u} as the eventual recipient of
> your changes is good, but then it seems like Git is lacking the
> "publish my changes to this other branch" concept.
>
> Am I missing something?  If there is something other than @{u} to
> represent this latter concept, I think `git push` should default to
> that instead.  But, at least with my current knowledge, that doesn't
> exist--without explicitly saying so--or treating @{u} as that branch.
> If there's a better way to do this, I'd love to hear it!

That's why we invented remote.pushdefault and branch.*.pushremote. When you say

  $ git push

it automatically goes to the right remote instead of going to the
place you fetched from. You can read up on how push.default interacts
with this setting too, although I always recommend push.default =
current.
--
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] completion: complete format.coverLetter

2014-01-07 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
>  contrib/completion/git-completion.bash | 1 +
>  1 file changed, 1 insertion(+)

Junio: Please push this part via 'maint' independently.
--
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 2/2] format-patch: introduce format.defaultTo

2014-01-07 Thread Ramkumar Ramachandra
John Szakmeister wrote:
> I guess it's not a good idea to
> set 'push.default' to 'upstream' in this triangle workflow then,
> otherwise the branch name being pushed to will be 'branch.*.merge'.
> Is that correct?  I just want to make sure I understand what's
> happening here.

push.default = upstream does not support triangular workflows. See
builtin/push.c:setup_push_upstream().

> Given this new found knowledge, I'm not sure it makes sense for `git
> status` to show me the status against the upstream vs. the publish
> location.  The latter makes a little more sense to me, though I see
> the usefulness of either one.

Currently, status information is only against @{u}; we haven't
invented a @{publish} yet.
--
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] format-patch: introduce branch.*.forkedFrom

2014-01-07 Thread Ramkumar Ramachandra
A very common workflow for preparing patches involves working off a
topic branch and generating patches against 'master' to send off to the
maintainer. However, a plain

  $ git format-patch -o outgoing

is a no-op on a topic branch, and the user has to remember to specify
'master' explicitly everytime. This problem is not unique to
format-patch; even a

  $ git rebase -i

is a no-op because the branch to rebase against isn't specified.

To tackle this problem, introduce branch.*.forkedFrom which can specify
the parent branch of a topic branch. Future patches will build
functionality around this new configuration variable.

Cc: Jeff King 
Cc: Junio C Hamano 
Signed-off-by: Ramkumar Ramachandra 
---
 Since -M, -C, -D are left in the argc, checking argc < 2 isn't
 sufficient.

 I wanted to get an early reaction before wiring up checkout and
 rebase.

 But I wanted to discuss the overall idea of the patch.
 builtin/log.c   | 21 +
 t/t4014-format-patch.sh | 20 
 2 files changed, 41 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index b97373d..525e696 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -674,6 +674,7 @@ static int thread;
 static int do_signoff;
 static const char *signature = git_version_string;
 static int config_cover_letter;
+static const char *config_base_branch;
 
 enum {
COVER_UNSET,
@@ -750,6 +751,22 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON : 
COVER_OFF;
return 0;
}
+   if (starts_with(var, "branch.")) {
+   const char *name = var + 7;
+   const char *subkey = strrchr(name, '.');
+   struct strbuf buf = STRBUF_INIT;
+
+   if (!subkey)
+   return 0;
+   strbuf_add(&buf, name, subkey - name);
+   if (branch_get(buf.buf) != branch_get(NULL))
+   return 0;
+   strbuf_release(&buf);
+   if (!strcmp(subkey, ".forkedfrom")) {
+   if (git_config_string(&config_base_branch, var, value))
+   return -1;
+   }
+   }
 
return git_log_config(var, value, cb);
 }
@@ -1324,6 +1341,10 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
die (_("--subject-prefix and -k are mutually exclusive."));
rev.preserve_subject = keep_subject;
 
+   if (argc < 2 && config_base_branch) {
+   argv[1] = config_base_branch;
+   argc++;
+   }
argc = setup_revisions(argc, argv, &rev, &s_r_opt);
if (argc > 1)
die (_("unrecognized argument: %s"), argv[1]);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 73194b2..2ea94af 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1370,4 +1370,24 @@ test_expect_success 'cover letter auto user override' '
test_line_count = 2 list
 '
 
+test_expect_success 'branch.*.forkedFrom matches' '
+   mkdir -p tmp &&
+   test_when_finished "rm -rf tmp;
+   git config --unset branch.rebuild-1.forkedFrom" &&
+
+   git config branch.rebuild-1.forkedFrom master &&
+   git format-patch -o tmp >list &&
+   test_line_count = 2 list
+'
+
+test_expect_success 'branch.*.forkedFrom does not match' '
+   mkdir -p tmp &&
+   test_when_finished "rm -rf tmp;
+   git config --unset branch.foo.forkedFrom" &&
+
+   git config branch.foo.forkedFrom master &&
+   git format-patch -o tmp >list &&
+   test_line_count = 0 list
+'
+
 test_done
-- 
1.8.5.2.234.gba2dde8.dirty

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


  1   2   3   4   5   6   7   8   9   10   >