Re: [PATCH v2 00/19] Multiparent diff tree-walker + combine-diff speedup

2014-02-25 Thread Kirill Smelkov
On Tue, Feb 25, 2014 at 06:43:24AM +0700, Duy Nguyen wrote:
 On Mon, Feb 24, 2014 at 11:21 PM, Kirill Smelkov k...@mns.spb.ru wrote:
  Hello up there.
 
  Here go combine-diff speedup patches in form of first reworking diff
  tree-walker to work in general case - when a commit have several parents, 
  not
  only one - we are traversing all 1+nparent trees in parallel.
 
  Then we are taking advantage of the new diff tree-walker for speeding up
  combine-diff, which for linux.git results in ~14 times speedup.
 
 I think there is another use case for this n-tree walker (but I'm not
 entirely sure yet as I haven't really read the series). In git-log
 (either with pathspec or --patch) we basically do this
 
 diff HEAD^ HEAD
 diff HEAD^^ HEAD^
 diff HEAD^^^ HEAD^^
 diff HEAD HEAD^^^
 ...
 
 so except HEAD (and the last commit), all commits' tree will be
 read/diff'd twice. With n-tree walker I think we may be able to diff
 them in batch to reduce extra processing: commit lists are split into
 16-commit blocks where 16 trees are fed to the new tree walker at the
 same time. I hope it would make git-log a bit faster (especially for
 -S). Maybe not much.

Thanks for commenting.

Unfortunately, as it is now, no, and I doubt savings will be
significant. The real speedup comes from the fact that for combined
diff, we can omit recursing into subdirectories, if we know some diff
D(commit,parent_i) is empty. Let me quote myself from

http://article.gmane.org/gmane.comp.version-control.git/242217

On Sun, Feb 16, 2014 at 12:08:29PM +0400, Kirill Smelkov wrote:
 On Fri, Feb 14, 2014 at 09:37:00AM -0800, Junio C Hamano wrote:
  I wonder if this machinery can be reused for log -m as well (or
  perhaps you do that already?).  After all, by performing a single
  parallel scan, you are gathering all the necessary information to
  let you pretend that you did N pairwise diff-tree.
 
 Unfortunately, as it is now, no, and let me explain why:
 
 The reason that is not true, is that we omit recursing into directories,
 if we know D(A,some-parent) for that path is empty. That means we don't
 calculate D(A,any-other-parents) for that path and subpaths.
 
 More structured description is that combined diff and log -m, which
 could be though as all diffs D(A,Pi) are different things:
 
 - the combined diff is D(A,B) generalization based on ^ (sets
   intersection) operator, and
 
 - log -m, aka all diffs is D(A,B) generalization based on v
   (sets union) operator.
 
 Intersection means, we can omit calculating parts from other sets, if we
 know some set does not have an element (remember don't recurse into
 subdirectories?), and unioning does not have this property.
 
 It does so happen, that ^ case (combine-diff) is more interesting,
 because in the end it allows to see new information - the diff a merge
 itself introduces. log -m does not have this property and is no more
 interesting to what plain diff(HEAD,HEAD^n) can provide - in other words
 it's just a convenience.
 
 Now, the diff tree-walker could be generalized once more, to allow
 clients specify, which diffs combination operator to use - intersection
 or unioning, but I doubt that for unioning case that would add
 significant speedup - we can't reduce any diff generation based on
 another diff and the only saving is that we traverse resulting commit
 tree once, but for some cases that could be maybe slower, say if result
 and some parents don't have a path and some parent does, we'll be
 recursing into that path and do more work compared to plain D(A,Pi) for
 Pi that lacks the path.
 
 In short: it could be generalized more, if needed, but I propose we
 first establish the ground with generalizing to just combine-diff.

besides

D(HEAD~,  HEAD)
D(HEAD~2, HEAD~)
...
D(HEAD~{n}, HEAD~{n-1})

is different even from log -m case as now there is no single commit
with several parents.

On a related note, while developing this n-tree walker, I've learned
that it is important to load trees in correct order. Quoting patch 18:

-   t1tree = fill_tree_descriptor(t1, old);
-   t2tree = fill_tree_descriptor(t2, new);
+   /*
+* load parents first, as they are probably already cached.
+*
+* ( log_tree_diff() parses commit-parent before calling here via
+*   diff_tree_sha1(parent, commit) )
+*/
+   for (i = 0; i  nparent; ++i)
+   tptree[i] = fill_tree_descriptor(tp[i], parents_sha1[i]);
+   ttree = fill_tree_descriptor(t, sha1);

so it loads parent's tree first. If we change this to be the other way,
i.e. load commit's tree first, and then parent's tree, there will be up
to 4% slowdown for whole plain `git log` (without -c).

So maybe what could be done to speedup plain log is for diff tree-walker
to populate some form of recently-loaded trees while walking, and drop
trees from will not-be used anymore commits - e.g. after doing
HEAD~..HEAD for next diff for HEAD~~..HEAD~ HEAD~ 

git submodule manpage does not document --checkout

2014-02-25 Thread Matthijs Kooijman
Hi,

it seems git submodule supports --checkout, which is also mentioned
indirectly in the manpage. However, the option itself is not mentioned
in the synopsis or detailed option list.

Gr.

Matthijs


signature.asc
Description: Digital signature


[PATCH v3] tag: support --sort=spec

2014-02-25 Thread Nguyễn Thái Ngọc Duy
--sort=version:refname (or --sort=v:refname for short) sorts tags as
if they are versions. --sort=-refname reverses the order (with or
without :version).

versioncmp() is copied from string/strverscmp.c in glibc commit
ee9247c38a8def24a59eb5cfb7196a98bef8cfdc, reformatted to Git coding
style. The implementation is under LGPL-2.1 and according to [1] I can
relicense it to GPLv2.

[1] http://www.gnu.org/licenses/gpl-faq.html#AllCompatibility

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 v3 tweaks the sorting syntax a bit (version:refname vs refname:version)
 and embed strverscmp() to git code (instead of relying on one from glibc).

 Documentation/git-tag.txt |  6 
 Makefile  |  1 +
 builtin/tag.c | 68 ++---
 cache.h   |  2 ++
 t/t7004-tag.sh| 43 
 versioncmp.c (new)| 85 +++
 6 files changed, 200 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 404257d..9b05931 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -95,6 +95,12 @@ OPTIONS
using fnmatch(3)).  Multiple patterns may be given; if any of
them matches, the tag is shown.
 
+--sort=type::
+   Sort in a specific order. Supported type is refname
+   (lexicographic order), version:refname or v:refname (tags
+   name are treated as versions). Prepend - to reverse sort
+   order.
+
 --column[=options]::
 --no-column::
Display tag listing in columns. See configuration variable
diff --git a/Makefile b/Makefile
index dddaf4f..16b00a5 100644
--- a/Makefile
+++ b/Makefile
@@ -884,6 +884,7 @@ LIB_OBJS += userdiff.o
 LIB_OBJS += utf8.o
 LIB_OBJS += varint.o
 LIB_OBJS += version.o
+LIB_OBJS += versioncmp.o
 LIB_OBJS += walker.o
 LIB_OBJS += wildmatch.o
 LIB_OBJS += wrapper.o
diff --git a/builtin/tag.c b/builtin/tag.c
index 74d3780..73b8a24 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -27,9 +27,16 @@ static const char * const git_tag_usage[] = {
NULL
 };
 
+#define STRCMP_SORT 0  /* must be zero */
+#define VERCMP_SORT 1
+#define SORT_MASK   0x7fff
+#define REVERSE_SORT0x8000
+
 struct tag_filter {
const char **patterns;
int lines;
+   int sort;
+   struct string_list tags;
struct commit_list *with_commit;
 };
 
@@ -166,7 +173,10 @@ static int show_reference(const char *refname, const 
unsigned char *sha1,
return 0;
 
if (!filter-lines) {
-   printf(%s\n, refname);
+   if (filter-sort)
+   string_list_append(filter-tags, refname);
+   else
+   printf(%s\n, refname);
return 0;
}
printf(%-15s , refname);
@@ -177,17 +187,39 @@ static int show_reference(const char *refname, const 
unsigned char *sha1,
return 0;
 }
 
+static int sort_by_version(const void *a_, const void *b_)
+{
+   const struct string_list_item *a = a_;
+   const struct string_list_item *b = b_;
+   return versioncmp(a-string, b-string);
+}
+
 static int list_tags(const char **patterns, int lines,
-   struct commit_list *with_commit)
+struct commit_list *with_commit, int sort)
 {
struct tag_filter filter;
 
filter.patterns = patterns;
filter.lines = lines;
+   filter.sort = sort;
filter.with_commit = with_commit;
+   memset(filter.tags, 0, sizeof(filter.tags));
+   filter.tags.strdup_strings = 1;
 
for_each_tag_ref(show_reference, (void *) filter);
-
+   if (sort) {
+   int i;
+   if ((sort  SORT_MASK) == VERCMP_SORT)
+   qsort(filter.tags.items, filter.tags.nr,
+ sizeof(struct string_list_item), sort_by_version);
+   if (sort  REVERSE_SORT)
+   for (i = filter.tags.nr - 1; i = 0; i--)
+   printf(%s\n, filter.tags.items[i].string);
+   else
+   for (i = 0; i  filter.tags.nr; i++)
+   printf(%s\n, filter.tags.items[i].string);
+   string_list_clear(filter.tags, 0);
+   }
return 0;
 }
 
@@ -427,6 +459,26 @@ static int parse_opt_points_at(const struct option *opt 
__attribute__((unused)),
return 0;
 }
 
+static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
+{
+   int *sort = opt-value;
+   if (*arg == '-') {
+   *sort = REVERSE_SORT;
+   arg++;
+   } else
+   *sort = STRCMP_SORT;
+   if (starts_with(arg, version:)) {
+   *sort |= VERCMP_SORT;
+   arg += 8;
+   } else if (starts_with(arg, v:)) {
+   

Re: `git stash pop` UX Problem

2014-02-25 Thread Holger Hellmuth

Am 24.02.2014 17:21, schrieb Matthieu Moy:

$ git add foo.txt
$ git status
On branch master
Changes to be committed:
   (use git reset HEAD file... to unstage)

 modified:   foo.txt


Maybe status should display a stash count if that count is  0, as this 
is part of the state of the repo.


$ git status
On branch master
Stashes: 1 --
Changes to be committed:
(use git reset HEAD file... to unstage)

  modified:   foo.txt

It would be in Omars example case a clear message that git kept the 
stash. And generally a reminder that there is still a stash around that 
might or might not be obsolete.



--
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 stash pop` UX Problem

2014-02-25 Thread Matthieu Moy
Holger Hellmuth hellm...@ira.uka.de writes:

 Am 24.02.2014 17:21, schrieb Matthieu Moy:
 $ git add foo.txt
 $ git status
 On branch master
 Changes to be committed:
(use git reset HEAD file... to unstage)

  modified:   foo.txt

 Maybe status should display a stash count if that count is  0, as
 this is part of the state of the repo.

Maybe it would help some users, but not me for example. My main use of
git stash is a safe replacement for git reset --hard: when I want to
discard changes, but keep them safe just in case.

So, my stash count is almost always 0, and I don't want to hear about
it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 stash pop` UX Problem

2014-02-25 Thread Omar Othman

Well, it's called `git stash` and not `git trash`... :-D

That's your own usage of it, but its main usage is different.

This is not a solution, but it's better than nothing and I second it.

On 25-02-14 13:33, Matthieu Moy wrote:

Holger Hellmuth hellm...@ira.uka.de writes:


Am 24.02.2014 17:21, schrieb Matthieu Moy:

$ git add foo.txt
$ git status
On branch master
Changes to be committed:
(use git reset HEAD file... to unstage)

  modified:   foo.txt

Maybe status should display a stash count if that count is  0, as
this is part of the state of the repo.

Maybe it would help some users, but not me for example. My main use of
git stash is a safe replacement for git reset --hard: when I want to
discard changes, but keep them safe just in case.

So, my stash count is almost always 0, and I don't want to hear about
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: `git stash pop` UX Problem

2014-02-25 Thread Omar Othman

Brandon:

Please note that what I am asking for is not always dropping the stash, 
but doing that *only* when the merge conflict is resolved. This is 
simply getting the whole command to be consistent. If you do `git stash 
pop` and it succeeds, the stash reference is dropped. If you do `git 
stash pop` and it succeeds *after resolving the merge conflict*, the 
stash reference is *not* dropped. This is *not* consistent and *is* a 
user experience problem. I'm not asking about dumbing git down by any means.


On 24-02-14 17:04, Brandon McCaig wrote:

Omar:

On Mon, Feb 24, 2014 at 3:32 AM, Omar Othman omar.oth...@booking.com wrote:

In general, whenever something a user should do, git always tells. So, for
example, when things go wrong with a merge, you have the option to abort.
When you are doing a rebase, git tells you to do git commit --amend, and
then git rebase --continue... and so on.

The point is: Because of this, git is expected to always instruct you on
what to do next in a multilevel operation, or instructing you what to do
when an operation has gone wrong.

Now comes the problem. When you do a git stash pop, and a merge conflict
happens, git correctly tells you to fix the problems and then git add to
resolve the conflict. But once that happens, and the internal status of git
tells you that there are no more problems (I have a prompt that tells me
git's internal status), the operation is not culminated by dropping the
stash reference, which what normally happens automatically after a git stash
pop. This has actually confused me for a lot of time, till I ran into a git
committer and asked him, and only then were I 100% confident that I did
nothing wrong and it is indeed a UX problem. I wasted a lot of time to know
why the operation is not completed as expected (since I trusted that git
just does the right thing), and it turned out that it is git's fault.

If this is accepted, please reply to this email and tell me to start working
on it. I've read the Documenation/SubmittingPatches guidelines, but I'll
appreciate also telling me where to base my change. My guess is maint, since
it's a bug in the sense of UX.

Unlike a merge, when you pop a stash that history is lost. If you
screw up the merge and the stash is dropped then there's generally no
reliable way to get it back. I think that it's correct behavior for
the stash to not be dropped if the merge conflicts. The user is
expected to manually drop the stash when they're done with it. It's
been a while since I've relied much on the stash (commits and branches
are more powerful to work with) so I'm not really familiar with what
help the UI gives when a conflict occurs now. Git's UI never really
expects the user to be negligent. It does help to hint to you what is
needed, but for the most part it still expects you to know what you're
doing and does what you say, not what you mean.

If there's any change that should be made it should be purely
providing more detailed instructions to the user about how to deal
with it. Either resolve the merge conflicts and git-add the
conflicting files, or use git-reset to either reset the index
(unstaging files nad clear) or reset index and working tree back to
HEAD. In general, I almost always git-reset after a git-stash pop
because I'm probably not ready to commit those changes yet and
generally want to still see those changes with git diff (without
--staged). Or perhaps just direct them to the appropriate sections of
the man pages.

I'm not really in favor of dumbing down Git in any way and I think
that any step in that direction would be for the worst... Software
should do what you say, not what you mean, because it's impossible to
reliably guess what you meant. When a git-stash pop operation fails
that might make the user rethink popping that stash. That's why it
becomes a manual operation to drop it if still desired. And unlike
git-reset --continue, which is explicitly the user saying it is fixed
and I accept the consequences, let's move on, there is no such option
to git-stash to acknowledge that the merge conflicts have been
resolved and you no longer need that stash (aside from git-stash drop,
of course). It's not a UI problem. It's maybe a documentation problem,
but again I'm not familiar with the current state of that.

/not a git dev...yet

Regards,




--
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 stash pop` UX Problem

2014-02-25 Thread Matthieu Moy
Omar Othman omar.oth...@booking.com writes:

 Brandon:

Please, don't top-post on this list. Look how other people answer to
each other and follow the use.

 Please note that what I am asking for is not always dropping the
 stash, but doing that *only* when the merge conflict is resolved. This
 is simply getting the whole command to be consistent. If you do `git
 stash pop` and it succeeds, the stash reference is dropped. If you do
 git stash pop` and it succeeds *after resolving the merge conflict*,
 the stash reference is *not* dropped. This is *not* consistent and
 *is* a user experience problem. I'm not asking about dumbing git down
 by any means.

Can you describe precisely what you would expect, e.g. what Git's output
should look like after such and such command?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 stash pop` UX Problem

2014-02-25 Thread Omar Othman



Please note that what I am asking for is not always dropping the
stash, but doing that *only* when the merge conflict is resolved. This
is simply getting the whole command to be consistent. If you do `git
stash pop` and it succeeds, the stash reference is dropped. If you do
git stash pop` and it succeeds *after resolving the merge conflict*,
the stash reference is *not* dropped. This is *not* consistent and
*is* a user experience problem. I'm not asking about dumbing git down
by any means.

Can you describe precisely what you would expect, e.g. what Git's output
should look like after such and such command?

Sure. This is my current command prompt (which shows git's internal status):

[omar_othman main (trunk*)]$

I do a git stash pop, which causes a merge conflict:

Auto-merging path/to/file.txt
CONFLICT (content): Merge conflict in path/to/file.txt

[omar_othman main (trunk|MERGING*)]$ vi path/to/file.txt
[omar_othman main (trunk|MERGING*)]$ git add path/to/file.txt
[omar_othman main (trunk*)]$

Note how the status message has changed to show that git is now happy. 
It is at that moment that the stash reference should be dropped (or the 
user (somehow) is notified to do that herself if desired), because this 
means that the popping operation has succeeded.

--
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 5/6] Document a bunch of functions defined in sha1_file.c

2014-02-25 Thread Michael Haggerty
On 02/24/2014 09:08 PM, Jonathan Nieder wrote:
 Michael Haggerty wrote:
 [...]  I've been documenting public functions in the
 header files above the declaration, and private ones where they are
 defined.  [...]

 Let me know if you think I've made it less helpful.
 
 In the present state of the codebase, where many important functions
 have no documentation or out-of-date documentation, the first place I
 look to understand a function is its point of definition.  So I do
 think that moving docs to the header file makes it less helpful.  I'd
 prefer a mass move in the opposite direction (from header files to the
 point of definition).
 
 On the other hand I don't feel strongly about it.

Jonathan,

I see your point.  But I'd rather that we, as a project, strive to make
our header files good tables of contents of the publicly-accessible
functionality, including decent documentation for each function.  I try
to add comments to everything I touch, and I wish other developers would
too.

[What we really need is a comment fascist who patrols patch submissions
making sure that they add docstrings for new functions.  If I only had
the time and the jackboots for it...]

So I'd rather leave the comments for public functions in the header
files.  But if other regular developers prefer that comments be by the
function definitions, of course I can live with that, too.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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 stash pop` UX Problem

2014-02-25 Thread Matthieu Moy
Omar Othman omar.oth...@booking.com writes:

 [omar_othman main (trunk|MERGING*)]$ git add path/to/file.txt
 [omar_othman main (trunk*)]$

 Note how the status message has changed to show that git is now happy.
 It is at that moment that the stash reference should be dropped

Dropping the stash on a git add operation would be really, really
weird...

 (or the user (somehow) is notified to do that herself if desired),
 because this means that the popping operation has succeeded.

But how would you expect to be notified?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: Can't use difftool to selectively revert changes

2014-02-25 Thread Robert Dailey
Sorry I'm going to go ahead and answer my own question:

$ git difftool $(git merge-base topic1 master) -- Path/SourceFile.cpp

I removed 'HEAD' from the command and now it picks up my changes and
compares to my working copy version (which is actually what I wanted).
I thought HEAD would point to my working copy version but that's
wrong.

Sorry for the superfluous post!

On Tue, Feb 25, 2014 at 9:22 AM, Robert Dailey rcdailey.li...@gmail.com wrote:
 I have a branch called topic1 that is based on 'master'. For a
 particular file in my topic branch, I want to revert some changes by
 using my diff tool. I do this by comparing the original revision of
 the file with HEAD like so:

 $ git difftool $(git merge-base topic1 master) HEAD -- Path/SourceFile.cpp

 When I make changes to the right side (HEAD) through my diff tool, and
 exit, the changes aren't picked up and applied to my working copy.
 Since I'm modifying HEAD, I think it should carry over the changes I
 make. Is this by design or a defect? I don't know how else to
 selectively revert changes to a file through my diff viewer.
--
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


Can't use difftool to selectively revert changes

2014-02-25 Thread Robert Dailey
I have a branch called topic1 that is based on 'master'. For a
particular file in my topic branch, I want to revert some changes by
using my diff tool. I do this by comparing the original revision of
the file with HEAD like so:

$ git difftool $(git merge-base topic1 master) HEAD -- Path/SourceFile.cpp

When I make changes to the right side (HEAD) through my diff tool, and
exit, the changes aren't picked up and applied to my working copy.
Since I'm modifying HEAD, I think it should carry over the changes I
make. Is this by design or a defect? I don't know how else to
selectively revert changes to a file through my diff viewer.
--
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


Git in GSoC 2014

2014-02-25 Thread Jeff King
I'm pleased to announce that Git has been accepted to this year's Google
Summer of Code.

Student proposals will start coming in on March 22. In the meantime
students will be reading our Ideas page[1] and enquiring about the
program on the mailing list and on irc. There are many ways that
existing git developers can help:

  - continue to add to and polish the Ideas page

  - interact with prospective students; besides responding to questions
on the list, hanging around on #git and #git-devel on freenode is
helpful

  - volunteer to mentor and/or rank student proposals. You can sign up
at http://google-melange.com, and request to be added as a mentor
for the git project.

We didn't discuss earlier whether we would have any specific
requirements for students during the proposal period (e.g., having a
patch accepted). It would be good to put together rules (or barring any
specific requirements, guidelines to help students put together a good
proposal) as soon as possible. Suggestions are welcome.

-Peff

[1] http://git.github.io/SoC-2014-Ideas.html
--
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 in GSoC 2014

2014-02-25 Thread Dmitry S. Dolzhenko

Hi.

I was just going to write an email about that I would like to 
participate in GSoC and contribute to Git project.


I don't have wide experience in C programming, but I could be start as a 
janitor. I found several tasks here 
https://git.wiki.kernel.org/index.php/Janitor. For example Refactor 
binary search functions. If this task is actual, I could be to start 
from 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: Git in GSoC 2014

2014-02-25 Thread Michael Haggerty
On 02/25/2014 04:41 PM, Jeff King wrote:
 I'm pleased to announce that Git has been accepted to this year's Google
 Summer of Code.

Cool!  Thanks to Peff and Thomas and Vicent and whomever else was
involved in getting our application done!  For those who don't know, the
application covers both Git core and libgit2.

 We didn't discuss earlier whether we would have any specific
 requirements for students during the proposal period (e.g., having a
 patch accepted). It would be good to put together rules (or barring any
 specific requirements, guidelines to help students put together a good
 proposal) as soon as possible. Suggestions are welcome.

Requiring students to submit a reasonable patch and follow up on review
comments seems like it would be a good way to filter out non-serious
students.  (I hesitate to require that the patch be accepted because it
can take quite a while for a patch to make it to master, despite of the
student's efforts.)

Does anybody know whether other organizations have had good experience
with criteria like that?  Does it chase *all* the applicants away?

If we wanted to impose such a hurdle, then we would definitely have to
make up a list of microprojects so that the students don't have to start
from nothing.  I imagine it shouldn't be too hard to find tiny projects
estimated at 10-30 minutes of actual work, which should be plenty
difficult for a student who also has to figure out how to check out the
code, conform to our coding standards, run the unit tests, create a
patch submission, etc.

If the reaction is positive to this idea then I volunteer to spend
several hours tomorrow looking for microprojects, and I suggest other
core developers do so as well.  They should presumably be submitted as
patches to the ideas repository [1].

What do you think?

Michael

[1] https://github.com/git/git.github.io

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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] tag: support --sort=spec

2014-02-25 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 versioncmp() is copied from string/strverscmp.c in glibc commit
 ee9247c38a8def24a59eb5cfb7196a98bef8cfdc, reformatted to Git coding
 style. The implementation is under LGPL-2.1 and according to [1] I can
 relicense it to GPLv2.

I'd propose this trivial change squashed in to record the above
in the file in question.

Thanks.

diff --git a/versioncmp.c b/versioncmp.c
index 18a9632..8f5a388 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -1,6 +1,13 @@
 #include git-compat-util.h
 
 /*
+ * versioncmp(): copied from string/strverscmp.c in glibc commit
+ * ee9247c38a8def24a59eb5cfb7196a98bef8cfdc, reformatted to Git coding
+ * style. The implementation is under LGPL-2.1 and Git relicenses it
+ * to GPLv2.
+ */
+
+/*
  * states: S_N: normal, S_I: comparing integral part, S_F: comparing
  * fractionnal parts, S_Z: idem but with leading Zeroes only
  */
--
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] builtin/blame.c::find_copy_in_blob: no need to scan for region end

2014-02-25 Thread Junio C Hamano
David Kastrup d...@gnu.org writes:

 The region end can be looked up just like its beginning.

 Signed-off-by: David Kastrup d...@gnu.org
 ---
  builtin/blame.c | 9 +
  1 file changed, 1 insertion(+), 8 deletions(-)

Yay, code reduction!  Thanks.

 diff --git a/builtin/blame.c b/builtin/blame.c
 index e44a6bb..96716dd 100644
 --- a/builtin/blame.c
 +++ b/builtin/blame.c
 @@ -939,7 +939,6 @@ static void find_copy_in_blob(struct scoreboard *sb,
 mmfile_t *file_p)
  {
   const char *cp;
 - int cnt;
   mmfile_t file_o;
   struct handle_split_cb_data d;
  
 @@ -950,13 +949,7 @@ static void find_copy_in_blob(struct scoreboard *sb,
*/
   cp = nth_line(sb, ent-lno);
   file_o.ptr = (char *) cp;
 - cnt = ent-num_lines;
 -
 - while (cnt  cp  sb-final_buf + sb-final_buf_size) {
 - if (*cp++ == '\n')
 - cnt--;
 - }
 - file_o.size = cp - file_o.ptr;
 + file_o.size = nth_line(sb, ent-lno + ent-num_lines) - cp;
  
   /*
* file_o is a part of final image we are annotating.
--
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] difftool: support repositories with .git-files

2014-02-25 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 Am 24.02.2014 17:55, schrieb Junio C Hamano:
 David Aguilar dav...@gmail.com writes:
 
 Modern versions of git submodule use .git-files to setup the
 submodule directory.  When run in a git submodule-created
 repository git difftool --dir-diff dies with the following
 error:

 $ git difftool -d HEAD~
 fatal: This operation must be run in a work tree
 diff --raw --no-abbrev -z HEAD~: command returned error: 128

 core.worktree is relative to the .git directory but the logic
 in find_worktree() does not account for it.

 Use `git rev-parse --show-toplevel` to find the worktree so that
 the dir-diff feature works inside a submodule.

 Reported-by: Gábor Lipták gabor.lip...@gmail.com
 Helped-by: Jens Lehmann jens.lehm...@web.de
 Helped-by: John Keeping j...@keeping.me.uk
 Signed-off-by: David Aguilar dav...@gmail.com
 ---
 
 Looks good; thanks.


 FWIW:
 Tested-by: Jens Lehmann jens.lehm...@web.de

 What about squashing this in to detect any future regressions?

 diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
 index 2418528..d86ad68 100755
 --- a/t/t7800-difftool.sh
 +++ b/t/t7800-difftool.sh
 @@ -434,4 +434,12 @@ test_expect_success PERL 'difftool --no-symlinks detects 
 conflict ' '
   )
  '

 +test_expect_success PERL 'difftool properly honours gitlink and 
 core.worktree' '
 + git submodule add ./. submod/ule 
 + (
 + cd submod/ule 
 + git difftool --tool=echo  --dir-diff --cached

In the context of this fix, finishing with 0 exit status may be all
we care about, but do we also care about things like in what
directory the tool is invoked in, what arguments and extra
environment settings (if any) it is given, and stuff like that?

In fact, the echo in the above is very misleading.  The test
relies on the fact that immediately after the submod/ule is cloned,
diff --cached does not have to call any tool backend---if you
modify some tracked file in its working tree and dropped --cached
on the command line, the command will fail with Huh?  I do not know
what 'echo' diff/merge backend is, no?

 + )
 +'
 +
  test_done
--
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 stash pop` UX Problem

2014-02-25 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Holger Hellmuth hellm...@ira.uka.de writes:

 Am 24.02.2014 17:21, schrieb Matthieu Moy:
 $ git add foo.txt
 $ git status
 On branch master
 Changes to be committed:
(use git reset HEAD file... to unstage)

  modified:   foo.txt

 Maybe status should display a stash count if that count is  0, as
 this is part of the state of the repo.

 Maybe it would help some users, but not me for example. My main use of
 git stash is a safe replacement for git reset --hard: when I want to
 discard changes, but keep them safe just in case.

 So, my stash count is almost always 0, and I don't want to hear about
 it.

status is about reminding the user what changes are already in the
index (i.e. what you would commit) and what changes are in the
working tree, from which you could further update the index with
(i.e. what you could commit).

One _could_ argue that stashed changes are what could be reflected
to the working tree and form the source of the latter, but my gut
feeling is that it is a rather weak argument.  At that point you are
talking about what you could potentially change in the working tree,
and the way to do so is not limited to stash pop (i.e. you can
git cherry-pick --no-commit $a_commit, or edit any file in the
working tree for that matter, with the same ease).

So, I tend to agree with you, while I do understand where I want to
know about what is in stash is coming from (and that is why we do
have git stash list command).

--
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] help: include list of aliases in git-help --all

2014-02-25 Thread Junio C Hamano
Joel Nothman joel.noth...@gmail.com writes:

 Git help --all had listed all git commands, but no configured aliases.
 This includes aliases as a separate listing, after commands in the main
 git directory and other $PATH directories.

... and why is this a good thing?


 Signed-off-by: Joel Nothman joel.nothman at gmail.com
 ---
  Documentation/git-help.txt |  4 +--
  builtin/help.c |  7 ++---
  help.c | 64 
 +++---
  help.h |  7 -
  4 files changed, 61 insertions(+), 21 deletions(-)

 diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
 index b21e9d7..c9fe791 100644
 --- a/Documentation/git-help.txt
 +++ b/Documentation/git-help.txt
 @@ -40,8 +40,8 @@ OPTIONS
  ---
  -a::
  --all::
 - Prints all the available commands on the standard output. This
 - option overrides any given command or guide name.
 + Prints all the available commands and aliases on the standard output.
 + This option overrides any given command or guide name.
  
  -g::
  --guides::
 diff --git a/builtin/help.c b/builtin/help.c
 index 1fdefeb..d1dfecd 100644
 --- a/builtin/help.c
 +++ b/builtin/help.c
 @@ -38,7 +38,7 @@ static int show_guides = 0;
  static unsigned int colopts;
  static enum help_format help_format = HELP_FORMAT_NONE;
  static struct option builtin_help_options[] = {
 - OPT_BOOL('a', all, show_all, N_(print all available commands)),
 + OPT_BOOL('a', all, show_all, N_(print all available commands and 
 aliases)),
   OPT_BOOL('g', guides, show_guides, N_(print list of useful 
 guides)),
   OPT_SET_INT('m', man, help_format, N_(show man page), 
 HELP_FORMAT_MAN),
   OPT_SET_INT('w', web, help_format, N_(show manual in web browser),
 @@ -453,6 +453,7 @@ int cmd_help(int argc, const char **argv, const char 
 *prefix)
   int nongit;
   const char *alias;
   enum help_format parsed_help_format;
 + struct cmdnames alias_cmds;
  
   argc = parse_options(argc, argv, prefix, builtin_help_options,
   builtin_help_usage, 0);
 @@ -461,8 +462,8 @@ int cmd_help(int argc, const char **argv, const char 
 *prefix)
   if (show_all) {
   git_config(git_help_config, NULL);
   printf(_(usage: %s%s), _(git_usage_string), \n\n);
 - load_command_list(git-, main_cmds, other_cmds);
 - list_commands(colopts, main_cmds, other_cmds);
 + load_commands_and_aliases(git-, main_cmds, other_cmds, 
 alias_cmds);
 + list_commands(colopts, main_cmds, other_cmds, alias_cmds);
   }
  
   if (show_guides)
 diff --git a/help.c b/help.c
 index df7d16d..3c14af4 100644
 --- a/help.c
 +++ b/help.c
 @@ -86,7 +86,7 @@ static void pretty_print_string_list(struct cmdnames *cmds,
   int i;
  
   for (i = 0; i  cmds-cnt; i++)
 - string_list_append(list, cmds-names[i]-name);
 + string_list_append(list, cmds-names[i]-name);
   /*
* always enable column display, we only consult column.*
* about layout strategy and stuff
 @@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
   exclude_cmds(other_cmds, main_cmds);
  }
  
 +static struct cmdnames aliases;
 +
 +static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
 +{
 + int i;
 + ALLOC_GROW(cmds-names, cmds-cnt + old-cnt, cmds-alloc);
 +
 + for (i = 0; i  old-cnt; i++)
 + cmds-names[cmds-cnt++] = old-names[i];
 + free(old-names);
 + old-cnt = 0;
 + old-names = NULL;
 +}
 +
 +static int load_aliases_cb(const char *var, const char *value, void *cb)
 +{
 + if (starts_with(var, alias.))
 + add_cmdname(aliases, var + 6, strlen(var + 6));
 + return 0;
 +}
 +
 +void load_commands_and_aliases(const char *prefix,
 + struct cmdnames *main_cmds,
 + struct cmdnames *other_cmds,
 + struct cmdnames *alias_cmds)
 +{
 + load_command_list(prefix, main_cmds, other_cmds);
 +
 + /* reuses global aliases from unknown command functionality */
 + git_config(load_aliases_cb, NULL);
 +
 + add_cmd_list(alias_cmds, aliases);
 + qsort(alias_cmds-names, alias_cmds-cnt,
 +   sizeof(*alias_cmds-names), cmdname_compare);
 + uniq(alias_cmds);
 + exclude_cmds(alias_cmds, main_cmds);
 + exclude_cmds(alias_cmds, other_cmds);
 +}
 +
  void list_commands(unsigned int colopts,
 -struct cmdnames *main_cmds, struct cmdnames *other_cmds)
 +struct cmdnames *main_cmds, struct cmdnames *other_cmds,
 +struct cmdnames *alias_cmds)
  {
   if (main_cmds-cnt) {
   const char *exec_path = git_exec_path();
 @@ -219,6 +259,13 @@ void list_commands(unsigned int colopts,
   pretty_print_string_list(other_cmds, colopts);
   putchar('\n');
   }
 +
 + if (alias_cmds-cnt) {
 + printf_ln(_(aliases 

Re: [PATCH] difftool: support repositories with .git-files

2014-02-25 Thread Jens Lehmann
Am 25.02.2014 18:02, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:
 
 Am 24.02.2014 17:55, schrieb Junio C Hamano:
 David Aguilar dav...@gmail.com writes:

 Modern versions of git submodule use .git-files to setup the
 submodule directory.  When run in a git submodule-created
 repository git difftool --dir-diff dies with the following
 error:

$ git difftool -d HEAD~
fatal: This operation must be run in a work tree
diff --raw --no-abbrev -z HEAD~: command returned error: 128

 core.worktree is relative to the .git directory but the logic
 in find_worktree() does not account for it.

 Use `git rev-parse --show-toplevel` to find the worktree so that
 the dir-diff feature works inside a submodule.

 Reported-by: Gábor Lipták gabor.lip...@gmail.com
 Helped-by: Jens Lehmann jens.lehm...@web.de
 Helped-by: John Keeping j...@keeping.me.uk
 Signed-off-by: David Aguilar dav...@gmail.com
 ---

 Looks good; thanks.


 FWIW:
 Tested-by: Jens Lehmann jens.lehm...@web.de

 What about squashing this in to detect any future regressions?

 diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
 index 2418528..d86ad68 100755
 --- a/t/t7800-difftool.sh
 +++ b/t/t7800-difftool.sh
 @@ -434,4 +434,12 @@ test_expect_success PERL 'difftool --no-symlinks 
 detects conflict ' '
  )
  '

 +test_expect_success PERL 'difftool properly honours gitlink and 
 core.worktree' '
 +git submodule add ./. submod/ule 
 +(
 +cd submod/ule 
 +git difftool --tool=echo  --dir-diff --cached
 
 In the context of this fix, finishing with 0 exit status may be all
 we care about, but do we also care about things like in what
 directory the tool is invoked in, what arguments and extra
 environment settings (if any) it is given, and stuff like that?

Sure. But I just intended to test the fix (and the test can easily
be extended by people who know more about difftool than I do).

 In fact, the echo in the above is very misleading.  The test
 relies on the fact that immediately after the submod/ule is cloned,
 diff --cached does not have to call any tool backend---if you
 modify some tracked file in its working tree and dropped --cached
 on the command line, the command will fail with Huh?  I do not know
 what 'echo' diff/merge backend is, no?

Right, using echo was not the best choice here. I used it to avoid
the dependency to meld in the example of the OP (maybe using true
as tool would have indicated that the tool is not important here,
but looking into this again a simple git difftool --dir-diff
without any further arguments also shows that the fix is working).

Aas mentioned above, I'm not familiar with difftool and just wanted
to share an easy way to test the fix. But I do not care too deeply
about this test, so feel free to ignore 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: `git stash pop` UX Problem

2014-02-25 Thread Stephen Leake
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Omar Othman omar.oth...@booking.com writes:

 [omar_othman main (trunk|MERGING*)]$ git add path/to/file.txt
 [omar_othman main (trunk*)]$

 Note how the status message has changed to show that git is now happy.
 It is at that moment that the stash reference should be dropped

 Dropping the stash on a git add operation would be really, really
 weird...

Why? That is when the merge conflicts are resolved, which is what
logically indicates that the stash is no longer needed, _if_ the merge
conflicts are related to the stash, which is true in this use case.

There are other uses for 'git add' that don't indicate that; we'd have
to be very careful to not throw away the stash at the wrong time.

 (or the user (somehow) is notified to do that herself if desired),
 because this means that the popping operation has succeeded.

 But how would you expect to be notified?

When 'git add' checks to see if all merge conflicts are now resolved,
and those merge conflicts were related to the stash, it can either pop
the stash, or issue a message telling the user it is now safe to do so.
We would need a config setting to indicate which to do.

Maybe that check is hard to do in general?

-- 
-- Stephe
--
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 stash pop` UX Problem

2014-02-25 Thread Stephen Leake
Junio C Hamano gits...@pobox.com writes:

 status is about reminding the user what changes are already in the
 index (i.e. what you would commit) and what changes are in the
 working tree, from which you could further update the index with
 (i.e. what you could commit).

I believe status should tell me everything git knows about the current
workspace in a resonably concise way. That includes the stash.

 One _could_ argue that stashed changes are what could be reflected
 to the working tree and form the source of the latter, but my gut
 feeling is that it is a rather weak argument.  At that point you are
 talking about what you could potentially change in the working tree,

No, I saved things in the stash on purpose. For example, I had changes
that were not ready to commit, but I wanted to do a merge from upstream.

There are workflows where the stash is not important; provide an option
to 'git status' that means ignore stash. 

 So, I tend to agree with you, while I do understand where I want to
 know about what is in stash is coming from (and that is why we do
 have git stash list command).

My Emacs front end currently checks both 'git status' and 'git stash
list' to build the status of the current workspace.

-- 
-- Stephe
--
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] help: include list of aliases in git-help --all

2014-02-25 Thread Joel Nothman
On 26 February 2014 06:15, Junio C Hamano gits...@pobox.com wrote:
 Joel Nothman joel.noth...@gmail.com writes:

 Git help --all had listed all git commands, but no configured aliases.
 This includes aliases as a separate listing, after commands in the main
 git directory and other $PATH directories.

 ... and why is this a good thing?

A fair question: I had considered it a conspicuous absence from the
list of commands in git help. After all, aliases are treated like
commands for a few purposes: they are executed as commands, they are
listed as possible command spelling corrections, and they are valid
arguments to git help. They are also like commands in that it is
possible to forget their name, or whether they are defined on a
particular workstation, and to hence want a listing. A user may also
not recall whether they had implemented a particular shortcut as an
alias or as a script (one may initially implement a script, and
realise an alias is sufficient; one may at first implement an alias
and realise it is insufficient, and that a script is needed).

In short, for many of the purposes in which one would seek git help
-a, there is no reason to *exclude* aliases from a list of commands
executed likewise.


 Signed-off-by: Joel Nothman joel.nothman at gmail.com
 ---
  Documentation/git-help.txt |  4 +--
  builtin/help.c |  7 ++---
  help.c | 64 
 +++---
  help.h |  7 -
  4 files changed, 61 insertions(+), 21 deletions(-)

 diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
 index b21e9d7..c9fe791 100644
 --- a/Documentation/git-help.txt
 +++ b/Documentation/git-help.txt
 @@ -40,8 +40,8 @@ OPTIONS
  ---
  -a::
  --all::
 - Prints all the available commands on the standard output. This
 - option overrides any given command or guide name.
 + Prints all the available commands and aliases on the standard output.
 + This option overrides any given command or guide name.

  -g::
  --guides::
 diff --git a/builtin/help.c b/builtin/help.c
 index 1fdefeb..d1dfecd 100644
 --- a/builtin/help.c
 +++ b/builtin/help.c
 @@ -38,7 +38,7 @@ static int show_guides = 0;
  static unsigned int colopts;
  static enum help_format help_format = HELP_FORMAT_NONE;
  static struct option builtin_help_options[] = {
 - OPT_BOOL('a', all, show_all, N_(print all available commands)),
 + OPT_BOOL('a', all, show_all, N_(print all available commands and 
 aliases)),
   OPT_BOOL('g', guides, show_guides, N_(print list of useful 
 guides)),
   OPT_SET_INT('m', man, help_format, N_(show man page), 
 HELP_FORMAT_MAN),
   OPT_SET_INT('w', web, help_format, N_(show manual in web browser),
 @@ -453,6 +453,7 @@ int cmd_help(int argc, const char **argv, const char 
 *prefix)
   int nongit;
   const char *alias;
   enum help_format parsed_help_format;
 + struct cmdnames alias_cmds;

   argc = parse_options(argc, argv, prefix, builtin_help_options,
   builtin_help_usage, 0);
 @@ -461,8 +462,8 @@ int cmd_help(int argc, const char **argv, const char 
 *prefix)
   if (show_all) {
   git_config(git_help_config, NULL);
   printf(_(usage: %s%s), _(git_usage_string), \n\n);
 - load_command_list(git-, main_cmds, other_cmds);
 - list_commands(colopts, main_cmds, other_cmds);
 + load_commands_and_aliases(git-, main_cmds, other_cmds, 
 alias_cmds);
 + list_commands(colopts, main_cmds, other_cmds, alias_cmds);
   }

   if (show_guides)
 diff --git a/help.c b/help.c
 index df7d16d..3c14af4 100644
 --- a/help.c
 +++ b/help.c
 @@ -86,7 +86,7 @@ static void pretty_print_string_list(struct cmdnames *cmds,
   int i;

   for (i = 0; i  cmds-cnt; i++)
 - string_list_append(list, cmds-names[i]-name);
 + string_list_append(list, cmds-names[i]-name);
   /*
* always enable column display, we only consult column.*
* about layout strategy and stuff
 @@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
   exclude_cmds(other_cmds, main_cmds);
  }

 +static struct cmdnames aliases;
 +
 +static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
 +{
 + int i;
 + ALLOC_GROW(cmds-names, cmds-cnt + old-cnt, cmds-alloc);
 +
 + for (i = 0; i  old-cnt; i++)
 + cmds-names[cmds-cnt++] = old-names[i];
 + free(old-names);
 + old-cnt = 0;
 + old-names = NULL;
 +}
 +
 +static int load_aliases_cb(const char *var, const char *value, void *cb)
 +{
 + if (starts_with(var, alias.))
 + add_cmdname(aliases, var + 6, strlen(var + 6));
 + return 0;
 +}
 +
 +void load_commands_and_aliases(const char *prefix,
 + struct cmdnames *main_cmds,
 + struct cmdnames *other_cmds,
 + struct cmdnames *alias_cmds)
 +{
 + load_command_list(prefix, main_cmds, other_cmds);
 +
 

Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-02-25 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes:

 Thinking some more about the tag_name issue, I realize that the other
 patch (Make request-pull able to take a refspec of form
 local:remote) broke another thing.

 The first patch pretty-printed the local branch-name, removing refs/
 and possibly heads/ from the local refname. So for a branch, it
 would ask people to just pull from the branch-name, and for a tag it
 would ask people to pull from tags/name, which is good policy. So if
 you had a tag called for-linus, it would say so (using
 tags/for-linus).

 But the local:remote syntax thing ends up breaking that nice feature.
 The old find_matching_refs would actually cause us to show the tags
 part if it existed on the remote, but that had become pointless and
 counter-productive with the first patch. But with the second patch,
 maybe we should reinstate that logic..

Sorry for back-burnering this topic so long.

I think the following does what you suggested in the message I am
responding to.

Now, hopefully the only thing we need is a documentation update and
the series should be ready to go.

-- 8 --
Subject: request-pull: resurrect pretty refname feature

When asking to fetch/pull a branch whose name is B or a tag whose
name is T, we used to show the command to run as:

git pull $URL B
git pull $URL tags/T

even when B and T were spelled in a more qualified way in order to
disambiguate, e.g. heads/B or refs/tags/T, but the recent update
lost this feature.  Resurrect it.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-request-pull.sh | 4 +++-
 t/t5150-request-pull.sh | 8 +++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 93b4135..b67513a 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -53,6 +53,8 @@ fi
 local=${3%:*}
 local=${local:-HEAD}
 remote=${3#*:}
+pretty_remote=${remote#refs/}
+pretty_remote=${pretty_remote#heads/}
 head=$(git symbolic-ref -q $local)
 head=${head:-$(git show-ref --heads --tags $local | cut -d' ' -f2)}
 head=${head:-$(git rev-parse --quiet --verify $local)}
@@ -124,7 +126,7 @@ git show -s --format='The following changes since commit %H:
 
 are available in the git repository at:
 ' $merge_base 
-echo   $url $remote 
+echo   $url $pretty_remote 
 git show -s --format='
 for you to fetch changes up to %H:
 
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 2622057..75d6b38 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -216,8 +216,14 @@ test_expect_success 'pull request format' '
git request-pull initial $downstream_url tags/full ../request
) 
request sed -nf fuzz.sed request.fuzzy 
-   test_i18ncmp expect request.fuzzy
+   test_i18ncmp expect request.fuzzy 
 
+   (
+   cd local 
+   git request-pull initial $downstream_url 
tags/full:refs/tags/full
+   ) request 
+   sed -nf fuzz.sed request request.fuzzy 
+   test_i18ncmp expect request.fuzzy
 '
 
 test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
--
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] help: include list of aliases in git-help --all

2014-02-25 Thread Junio C Hamano
Joel Nothman joel.noth...@gmail.com writes:

 arguments to git help. They are also like commands in that it is
 possible to forget their name, or whether they are defined on a
 particular workstation, and to hence want a listing.

I did envision that it would be useful for the last case, but then
the users would be helped even more if they can get a list of ONLY
aliases, not buried in many standard commands they already know
about.

In other words, I was not fundamentally opposed to *a* way to get a
list that includes aliases, but I was not convinced if it is a good
idea to *change* the output, which people knew would consist of
commands but not aliases, to suddenly start including aliases.
--
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] difftool: support repositories with .git-files

2014-02-25 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 +test_expect_success PERL 'difftool properly honours gitlink and 
 core.worktree' '
 +   git submodule add ./. submod/ule 
 +   (
 +   cd submod/ule 
 +   git difftool --tool=echo  --dir-diff --cached
 
 In the context of this fix, finishing with 0 exit status may be all
 we care about, but do we also care about things like in what
 directory the tool is invoked in, what arguments and extra
 environment settings (if any) it is given, and stuff like that?

 Sure. But I just intended to test the fix (and the test can easily
 be extended by people who know more about difftool than I do).

Yes, we need to start somewhere and I'd agree that it was a good
starting point.

 Right, using echo was not the best choice here. I used it to avoid
 the dependency to meld...

Perhaps like this then?  This is an a monkey sees what
difftool_test_setup does and then mimics patch ;-).

 t/t7800-difftool.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 2418528..595f808 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -434,4 +434,17 @@ test_expect_success PERL 'difftool --no-symlinks detects 
conflict ' '
)
 '
 
+test_expect_success PERL 'difftool properly honours gitlink and core.worktree' 
'
+   git submodule add ./. submod/ule 
+   (
+   cd submod/ule 
+   git config diff.tool checktrees 
+   git config difftool.checktrees.cmd '\''
+   test -d $LOCAL  test -d $REMOTE
+   '\'' 
+   echo further file 
+   git difftool --tool=checktrees --dir-diff
+   )
+'
+
 test_done
--
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 stash pop` UX Problem

2014-02-25 Thread Junio C Hamano
Stephen Leake stephen_le...@stephe-leake.org writes:

 One _could_ argue that stashed changes are what could be reflected
 to the working tree and form the source of the latter, but my gut
 feeling is that it is a rather weak argument.  At that point you are
 talking about what you could potentially change in the working tree,

 No, I saved things in the stash on purpose. For example, I had changes
 that were not ready to commit, but I wanted to do a merge from upstream.

I often save things by running git diff P.diff on purpose.
Should git status read these patches and tell me what paths I
could change in the working tree by applying it?  Where does it end?

 There are workflows where the stash is not important; provide an option
 to 'git status' that means ignore stash. 

How is that different to tell those who want to know what are in the
stash to type git stash list when they want to learn that
information?
--
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 stash pop` UX Problem

2014-02-25 Thread Junio C Hamano
Stephen Leake stephen_le...@stephe-leake.org writes:

 Dropping the stash on a git add operation would be really, really
 weird...

 Why? That is when the merge conflicts are resolved, which is what
 logically indicates that the stash is no longer needed,...

Not necessarily.  Imagine a case where you used stash to quickly
save away a tangled mess that was not ready for a logically single
commit and now you are in the process of creating the first commit
by applying it piece-by-piece to create multiple resulting ones.
After you commit the result, you would still want to keep the parts
of that stashed change you did not include in the first commit so
that you can go back, no?

You may run git add, but that does not say anything about what you
are going to use the rest of the stash for.  Not even git commit
may be a good enough sign.

--
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] help: include list of aliases in git-help --all

2014-02-25 Thread Joel Nothman
On 26 February 2014 08:51, Junio C Hamano gits...@pobox.com wrote:
 Joel Nothman joel.noth...@gmail.com writes:

 arguments to git help. They are also like commands in that it is
 possible to forget their name, or whether they are defined on a
 particular workstation, and to hence want a listing.

 I did envision that it would be useful for the last case, but then
 the users would be helped even more if they can get a list of ONLY
 aliases, not buried in many standard commands they already know
 about.

The list is partitioned. It is partitioned already between
git-installed commands and others on the path. This patch adds a third
partition when required. So they *do* see only aliases... after all
the commands.

Note also that any command on the path will override an alias with the
same name. So in order to list (effective) aliases, you need to
calculate the list of commands as well. If someone defines an alias
overridden by a command, git help -a now makes that apparent by
excluding the alias and including the command above it, while `git
config --get-regexp ^alias` does not.

 In other words, I was not fundamentally opposed to *a* way to get a
 list that includes aliases, but I was not convinced if it is a good
 idea to *change* the output, which people knew would consist of
 commands but not aliases, to suddenly start including aliases.

I don't think this will concern most users for whom aliases are
non-existent, and hence no section will be shown.
--
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 07/25] reflog: avoid constructing .lock path with git_path

2014-02-25 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 git_path() soon understands the path given to it. Some paths abc may
 become def while other ghi may become ijk. We don't want
 git_path() to interfere with .lock path construction. Concatenate
 .lock after the path has been resolved by git_path() so if abc
 becomes def, we'll have def.lock, not ijk.

Hmph.  I am not sure if the above is readable (or if I am reading it
correctly).

If abc becomes def, it would take deliberate work to make
abc.lock into ijk, and it would be natural to expect that
abc.lock would become def.lock without any fancy trick, no?


 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  builtin/reflog.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/builtin/reflog.c b/builtin/reflog.c
 index 852cff6..ccf2cf6 100644
 --- a/builtin/reflog.c
 +++ b/builtin/reflog.c
 @@ -372,7 +372,7 @@ static int expire_reflog(const char *ref, const unsigned 
 char *sha1, int unused,
   if (!file_exists(log_file))
   goto finish;
   if (!cmd-dry_run) {
 - newlog_path = git_pathdup(logs/%s.lock, ref);
 + newlog_path = mkpathdup(%s.lock, log_file);
   cb.newlog = fopen(newlog_path, w);
   }
--
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] help: include list of aliases in git-help --all

2014-02-25 Thread Philip Oakley

From: Joel Nothman joel.noth...@gmail.com

On 26 February 2014 06:15, Junio C Hamano gits...@pobox.com wrote:

Joel Nothman joel.noth...@gmail.com writes:

Git help --all had listed all git commands, but no configured 
aliases.
This includes aliases as a separate listing, after commands in the 
main

git directory and other $PATH directories.


... and why is this a good thing?


A fair question: I had considered it a conspicuous absence from the
list of commands in git help.


Surely one alternative would be to add an --alias or --aliases option to 
the help command so the user can chose which ones s/he desires to be 
helped about.


At least that's the way I did it with the --guides option.


 After all, aliases are treated like
commands for a few purposes: they are executed as commands, they are
listed as possible command spelling corrections, and they are valid
arguments to git help. They are also like commands in that it is
possible to forget their name, or whether they are defined on a
particular workstation, and to hence want a listing. A user may also
not recall whether they had implemented a particular shortcut as an
alias or as a script (one may initially implement a script, and
realise an alias is sufficient; one may at first implement an alias
and realise it is insufficient, and that a script is needed).

In short, for many of the purposes in which one would seek git help
-a, there is no reason to *exclude* aliases from a list of commands
executed likewise.



Signed-off-by: Joel Nothman joel.nothman at gmail.com
---
 Documentation/git-help.txt |  4 +--
 builtin/help.c |  7 ++---
 help.c | 64 
+++---

 help.h |  7 -
 4 files changed, 61 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index b21e9d7..c9fe791 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -40,8 +40,8 @@ OPTIONS
 ---
 -a::
 --all::
- Prints all the available commands on the standard output. This
- option overrides any given command or guide name.
+ Prints all the available commands and aliases on the standard 
output.

+ This option overrides any given command or guide name.

 -g::
 --guides::
diff --git a/builtin/help.c b/builtin/help.c
index 1fdefeb..d1dfecd 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -38,7 +38,7 @@ static int show_guides = 0;
 static unsigned int colopts;
 static enum help_format help_format = HELP_FORMAT_NONE;
 static struct option builtin_help_options[] = {
- OPT_BOOL('a', all, show_all, N_(print all available 
commands)),
+ OPT_BOOL('a', all, show_all, N_(print all available 
commands and aliases)),
  OPT_BOOL('g', guides, show_guides, N_(print list of useful 
guides)),
  OPT_SET_INT('m', man, help_format, N_(show man page), 
HELP_FORMAT_MAN),
  OPT_SET_INT('w', web, help_format, N_(show manual in web 
browser),
@@ -453,6 +453,7 @@ int cmd_help(int argc, const char **argv, const 
char *prefix)

  int nongit;
  const char *alias;
  enum help_format parsed_help_format;
+ struct cmdnames alias_cmds;

  argc = parse_options(argc, argv, prefix, builtin_help_options,
  builtin_help_usage, 0);
@@ -461,8 +462,8 @@ int cmd_help(int argc, const char **argv, const 
char *prefix)

  if (show_all) {
  git_config(git_help_config, NULL);
  printf(_(usage: %s%s), _(git_usage_string), \n\n);
- load_command_list(git-, main_cmds, other_cmds);
- list_commands(colopts, main_cmds, other_cmds);
+ load_commands_and_aliases(git-, main_cmds, 
other_cmds, alias_cmds);
+ list_commands(colopts, main_cmds, other_cmds, 
alias_cmds);

  }

  if (show_guides)
diff --git a/help.c b/help.c
index df7d16d..3c14af4 100644
--- a/help.c
+++ b/help.c
@@ -86,7 +86,7 @@ static void pretty_print_string_list(struct 
cmdnames *cmds,

  int i;

  for (i = 0; i  cmds-cnt; i++)
- string_list_append(list, cmds-names[i]-name);
+ string_list_append(list, cmds-names[i]-name);
  /*
   * always enable column display, we only consult column.*
   * about layout strategy and stuff
@@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
  exclude_cmds(other_cmds, main_cmds);
 }

+static struct cmdnames aliases;
+
+static void add_cmd_list(struct cmdnames *cmds, struct cmdnames 
*old)

+{
+ int i;
+ ALLOC_GROW(cmds-names, cmds-cnt + old-cnt, cmds-alloc);
+
+ for (i = 0; i  old-cnt; i++)
+ cmds-names[cmds-cnt++] = old-names[i];
+ free(old-names);
+ old-cnt = 0;
+ old-names = NULL;
+}
+
+static int load_aliases_cb(const char *var, const char *value, void 
*cb)

+{
+ if (starts_with(var, alias.))
+ add_cmdname(aliases, var + 6, strlen(var + 6));
+ return 0;
+}
+
+void load_commands_and_aliases(const char *prefix,
+  

Re: [PATCH] help: include list of aliases in git-help --all

2014-02-25 Thread Junio C Hamano
Joel Nothman joel.noth...@gmail.com writes:

 Git help --all had listed all git commands, but no configured aliases.
 This includes aliases as a separate listing, after commands in the main
 git directory and other $PATH directories.

 Signed-off-by: Joel Nothman joel.nothman at gmail.com
 ---

Thanks.

 diff --git a/help.c b/help.c
 index df7d16d..3c14af4 100644
 --- a/help.c
 +++ b/help.c
 @@ -86,7 +86,7 @@ static void pretty_print_string_list(struct cmdnames *cmds,
   int i;
  
   for (i = 0; i  cmds-cnt; i++)
 - string_list_append(list, cmds-names[i]-name);
 + string_list_append(list, cmds-names[i]-name);

Why?

 @@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
   exclude_cmds(other_cmds, main_cmds);
  }
  
 +static struct cmdnames aliases;

Instead of using a static global variable, perhaps make this an
on-stack variable in load_commands_and_aliases() that is passed as a
callback parameter to load_aliases_cb() thru git_config()?

 +static int load_aliases_cb(const char *var, const char *value, void *cb)
 +{

That is, cb here is the second parameter you gave to git_config().

  void list_commands(unsigned int colopts,
 -struct cmdnames *main_cmds, struct cmdnames *other_cmds)
 +struct cmdnames *main_cmds, struct cmdnames *other_cmds,
 +struct cmdnames *alias_cmds)
  {
   if (main_cmds-cnt) {
   const char *exec_path = git_exec_path();
 @@ -219,6 +259,13 @@ void list_commands(unsigned int colopts,
   pretty_print_string_list(other_cmds, colopts);
   putchar('\n');
   }
 +
 + if (alias_cmds-cnt) {
 + printf_ln(_(aliases defined in git configuration));

This will not break the use of git help -a in our completion
script, because it ignores anything that does not begin with two SP
followed by alphanumerics.

It may however break scripts that read from help -a done by other
people that may remove the lines in the output that are known to
them as not names of commands (i.e. available git commands...  and
git commands avaliable elsewhere...)---they haven't seen this new
string and would not know that this line must be skipped.

Other than that, looks reasonably cleanly done.  We'd need a test to
cover this so that other people will not break it in future patches.


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


What's cooking in git.git (Feb 2014, #07; Tue, 25)

2014-02-25 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

The tip of 'next' has been rewound.  There are healthy number of
topics in there that have been well-cooked during the 1.9.0
development cycle that can graduate to 'master' (see the announce in
http://article.gmane.org/gmane.comp.version-control.git/242658).
After they are merged to 'master', I'm planning to go back to the
list archive to pick up patches I may have missed in the meantime.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* ak/gitweb-fit-image (2014-02-20) 1 commit
 - gitweb: Avoid overflowing page body frame with large images

 Instead of allowing an img to be shown in whatever size, force
 scaling it to fit on the page with max-height/max-width css style
 attributes.

 Will merge to 'next'.


* da/difftool-git-files (2014-02-24) 1 commit
 - difftool: support repositories with .git-files

 git difftool misbehaved when the repository is bound to the
 working tree with the .git file mechanism, where a textual
 file .git tells us where it is.

 Will merge to 'next'.


* jk/commit-dates-parsing-fix (2014-02-24) 5 commits
 - log: do not segfault on gmtime errors
 - log: handle integer overflow in timestamps
 - date: check date overflow against time_t
 - fsck: report integer overflow in author timestamps
 - t4212: test bogus timestamps with git-log

 Will merge to 'next'.


* jk/diff-filespec-cleanup (2014-02-24) 1 commit
 - diffcore.h: be explicit about the signedness of is_binary

 Will merge to 'next' and then to 'master' and 'maint'.


* jk/remote-pushremote-config-reading (2014-02-24) 1 commit
 - remote: handle pushremote config in any order

 Will merge to 'next'.


* jk/repack-pack-keep-objects (2014-02-24) 1 commit
 - repack: add `repack.packKeepObjects` config var
 (this branch uses jk/pack-bitmap.)

 Names?


* jm/stash-doc-k-for-keep (2014-02-24) 1 commit
 - stash doc: mention short form -k in save description

 Will merge to 'next'.


* jn/am-doc-hooks (2014-02-24) 1 commit
 - am doc: add a pointer to relevant hooks

 Will merge to 'next'.


* mh/object-code-cleanup (2014-02-24) 4 commits
 - sha1_file.c: document a bunch of functions defined in the file
 - sha1_file_name(): declare to return a const string
 - find_pack_entry(): document last_found_pack
 - replace_object: use struct members instead of an array

 Will merge to 'next'.


* nd/i18n-progress (2014-02-24) 1 commit
 - i18n: mark all progress lines for translation

 Will merge to 'next'.


* nd/sha1-file-delta-stack-leakage-fix (2014-02-24) 1 commit
 - sha1_file: fix delta_stack memory leak in unpack_entry

 Will merge to 'next' and then to 'master' and 'maint'.


* tc/commit-dry-run-exit-status-tests (2014-02-24) 1 commit
 - demonstrate git-commit --dry-run exit code behaviour

--
[Stalled]

* kb/fast-hashmap-pack-struct (2014-02-24) 1 commit
 - hashmap.h: make sure map entries are tightly packed
 (this branch uses kb/fast-hashmap.)

 I am inclined to drop this; an alternative is to replace it with
 the more portable one that uses #pragma, which I am willing to
 try doing so on 'pu', though.


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

 Will hold.


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


* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
 - merge: drop unused arg from abort_commit method signature
 - merge: make prepare_to_commit responsible for write_merge_state
 - t7505: ensure cleanup after hook blocks merge
 - t7505: add missing 

 Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
 run during git merge.  The log message stresses too much on one
 hook, prepare-commit-msg, but it would equally apply to other hooks
 like post-merge, I think.

 Waiting for a reroll.


* jl/submodule-recursive-checkout (2013-12-26) 5 commits
 - Teach checkout to recursively checkout submodules
 - submodule: teach unpack_trees() to 

Re: `git stash pop` UX Problem

2014-02-25 Thread brian m. carlson
On Tue, Feb 25, 2014 at 01:33:56PM +0100, Matthieu Moy wrote:
 Holger Hellmuth hellm...@ira.uka.de writes:
  Maybe status should display a stash count if that count is  0, as
  this is part of the state of the repo.
 
 Maybe it would help some users, but not me for example. My main use of
 git stash is a safe replacement for git reset --hard: when I want to
 discard changes, but keep them safe just in case.
 
 So, my stash count is almost always 0, and I don't want to hear about
 it.

I concur with this.  Sometimes the stashed changes are remnants of a
small hack or a very brief start to an aborted project that I stashed
when I needed to change branches.  I figure that they might be useful in
the future, but I don't care about them right now.  I may pick them up,
I may not, but I certainly don't want to be reminded of them constantly.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: `git stash pop` UX Problem

2014-02-25 Thread Simon Ruderich
On Mon, Feb 24, 2014 at 05:21:40PM +0100, Matthieu Moy wrote:
 One easy thing to do OTOH would be to show a hint at the end of git
 stash pop's output, like

I think that's a good idea. It makes it obvious that Git has kept
the stash and that the user should drop it when he's done - if he
wants to.

 $ git stash pop
 Auto-merging foo.txt
 CONFLICT (content): Merge conflict in foo.txt
 'stash pop' failed. Please, resolve the conflicts manually. The stash
 was not dropped in case you need to restart the operation. When you are
 done resolving the merge, you may run the following to drop the stash:

   git stash drop

Maybe just the following to keep the output on a single line:

Use 'git stash drop' to remove the stash after resolving the conflicts.

But maybe that's too short as it doesn't mention explicitly, that
the stash was kept.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
--
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 10/25] Add new environment variable $GIT_COMMON_DIR

2014-02-25 Thread Eric Sunshine
On Tue, Feb 18, 2014 at 8:39 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 This variable is intended to support multiple working directories
 attached to a repository. Such a repository may have a main working
 directory, created by either git init or git clone and one or more
 linked working directories. These working directories and the main
 repository share the same repository directory.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
 diff --git a/Documentation/git.txt b/Documentation/git.txt
 index 02bbc08..2c4a055 100644
 --- a/Documentation/git.txt
 +++ b/Documentation/git.txt
 @@ -773,6 +773,14 @@ Git so take care if using Cogito etc.
 an explicit repository directory set via 'GIT_DIR' or on the
 command line.

 +'GIT_COMMON_DIR'::
 +   If this variable is set to a path, non-worktree files that are
 +   normally in $GIT_DIR will be taken from this path
 +   instead. Worktree-specific files such as HEAD or index are
 +   taken from $GIT_DIR. This variable has lower precedence than
 +   other path variables such as GIT_INDEX_FILE,
 +   GIT_OBJECT_DIRECTORY...

For a person not familiar with git checkout --to or its underlying
implementation, this description may be lacking. Such a reader may be
left wondering about GIT_COMMON_DIR's overall purpose, and when and
how it should be used. Perhaps it would make sense to talk a bit about
git checkout --to here?

  Git Commits
  ~~~
  'GIT_AUTHOR_NAME'::
--
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 stash pop` UX Problem

2014-02-25 Thread Omar Othman



[omar_othman main (trunk|MERGING*)]$ git add path/to/file.txt
[omar_othman main (trunk*)]$

Note how the status message has changed to show that git is now happy.
It is at that moment that the stash reference should be dropped

Dropping the stash on a git add operation would be really, really
weird...


(or the user (somehow) is notified to do that herself if desired),
because this means that the popping operation has succeeded.

But how would you expect to be notified?

Answering the last question, your previous comments are fine with me:

If there's any change that should be made it should be purely
providing more detailed instructions to the user about how to deal
with it.

Yes, there may be room for improvement, but that does not seem so easy.
Today, we have:

$ git stash pop
Auto-merging foo.txt
CONFLICT (content): Merge conflict in foo.txt

$ git status
On branch master
Unmerged paths:
   (use git reset HEAD file... to unstage)
   (use git add file... to mark resolution)

 both modified:  foo.txt

= The advices shown here are OK. Then:

$ git add foo.txt
$ git status
On branch master
Changes to be committed:
   (use git reset HEAD file... to unstage)

 modified:   foo.txt

= here, git status could have hinted the user you may now run 'git
stash drop' if you are satisfied with your merge.

Though I don't know why you think this is important:

Now, the real question is: when would Git stop showing this advice. I
don't see a real way to answer this, and I'd rather avoid doing just a
guess.
If it is really annoying for the user, we can just have a configuration 
parameter to switch this message on/off. I don't know whether git has 
such customizations (in general) currently.


This is very useful (maybe we can agree on wording later):

One easy thing to do OTOH would be to show a hint at the end of git
stash pop's output, like

$ git stash pop
Auto-merging foo.txt
CONFLICT (content): Merge conflict in foo.txt
'stash pop' failed. Please resolve the conflicts manually. The stash
was not dropped in case you need to restart the operation. When you are
done resolving the merge, you may run the following to drop the stash reference:

   git stash drop


--
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 stash pop` UX Problem

2014-02-25 Thread Omar Othman



Am 24.02.2014 17:21, schrieb Matthieu Moy:

$ git add foo.txt
$ git status
On branch master
Changes to be committed:
   (use git reset HEAD file... to unstage)

 modified:   foo.txt


Maybe status should display a stash count if that count is  0, as 
this is part of the state of the repo.


$ git status
On branch master
Stashes: 1 --
Changes to be committed:
(use git reset HEAD file... to unstage)

  modified:   foo.txt

It would be in Omars example case a clear message that git kept the 
stash. And generally a reminder that there is still a stash around 
that might or might not be obsolete.
Again, the same comment: If there is a way to customize git's messages 
by turning them on/off (or, even cooler, the ability to change their 
wording) then this is also a nice option to have and we can turn it off 
by default if we find that most people (here at least) don't like it. I 
don't know whether you guys have discussed this option before (or does 
it exist? I doubt, but I don't know), because having such an option (the 
ability to turn messages on/off or change their wording and what 
internal status information they manifest) will really resolve all kinds 
of such potential conflicts of preferences. Even cooler, people will be 
able to change the wording to their native languages for example.

--
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 stash pop` UX Problem

2014-02-25 Thread Omar Othman



Matthieu Moy matthieu@grenoble-inp.fr writes:


Holger Hellmuth hellm...@ira.uka.de writes:


Am 24.02.2014 17:21, schrieb Matthieu Moy:

$ git add foo.txt
$ git status
On branch master
Changes to be committed:
(use git reset HEAD file... to unstage)

  modified:   foo.txt

Maybe status should display a stash count if that count is  0, as
this is part of the state of the repo.

Maybe it would help some users, but not me for example. My main use of
git stash is a safe replacement for git reset --hard: when I want to
discard changes, but keep them safe just in case.

So, my stash count is almost always 0, and I don't want to hear about
it.

status is about reminding the user what changes are already in the
index (i.e. what you would commit) and what changes are in the
working tree, from which you could further update the index with
(i.e. what you could commit).

One _could_ argue that stashed changes are what could be reflected
to the working tree and form the source of the latter, but my gut
feeling is that it is a rather weak argument.  At that point you are
talking about what you could potentially change in the working tree,
and the way to do so is not limited to stash pop (i.e. you can
git cherry-pick --no-commit $a_commit, or edit any file in the
working tree for that matter, with the same ease).

So, I tend to agree with you, while I do understand where I want to
know about what is in stash is coming from (and that is why we do
have git stash list command).
Same comment. Everyone will have his own opinion. As long as the 
messages are not customizable, we can debate for hours and everybody has 
a valid point.

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