Re: Command-line git Vs IDE+plugin?

2013-11-19 Thread Konstantin Khomoutov
On Mon, 18 Nov 2013 18:42:26 +0100
Philippe Vaucher philippe.vauc...@gmail.com wrote:

[...]
 When they understand git reasonably (or if they are not lazy people
 and willing to learn), then show them full integrations like
 TortoiseGit (or probably the Netbeans plugin), which are nice when
 everything works but you have to know console git to fix things or
 simply to be aware of their limitations.

When it comes to Git on Windows with a GUI front-end, I would recommend
to take a look at Git Extensions [1] as well.

1. http://code.google.com/p/gitextensions/
--
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: Command-line git Vs IDE+plugin?

2013-11-19 Thread Thomas Koch
On Monday, November 18, 2013 06:11:54 PM Matthieu Moy wrote:
 Hi,
 
 I'm normally an Emacs+command-line user, but I also use Eclipse or
 Netbeans on some projects. I tried using EGit and the Netbeans plugin
 for Git, but found the GUI both more comlex and less powerful than the
 command-line. I end-up using command-line git in a terminal, outside the
 IDE (and do a refresh in the IDE after commands that modify the
 worktree). Obviously, being a long-time command-line user, I'm rather
 heavily biaised ;-).
 
 I was wondering whether others had similar (or not) experience. In
 particular, as a teacher, I'm wondering whether I should push my
 students towards the GUI in the IDE, or advise them to keep using the
 command-line (we teach them git with the command-line first anyway, but
 after a year of practice, we may want to show them the GUI equivalent).

I'm a software engineer now with an education as a high school teacher. From a 
theoretical point of view it's preferable to avoid any abstraction done by a 
GUI and use commandline Git. Only gitk is useful to have a visual _feedback_ 
of the actions done on the commandline.

But also from experience I can tell that without exception everybody whom I 
teached Git understood it only after being introduced to the basic concepts of 
Git and how to inspect and operate them on the commandline. Others told me 
from similar experiences.

Those concepts are:

- hashes
- content adressable storage
- blops being referenced by trees being referenced by commits

My collegues meanwhile dumped their graphical Git tool because they learned 
that they have better control over Git when using it from the commandline.

Regards, Thomas Koch
--
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: Command-line git Vs IDE+plugin?

2013-11-19 Thread Noufal Ibrahim KV
Matthieu Moy matthieu@grenoble-inp.fr writes:


[...]

 I was wondering whether others had similar (or not) experience. In
 particular, as a teacher, I'm wondering whether I should push my
 students towards the GUI in the IDE, or advise them to keep using the
 command-line (we teach them git with the command-line first anyway,
 but after a year of practice, we may want to show them the GUI
 equivalent).

[...]

I teach git professionally and I do so using only the command line
interface. This allows me to explain the underlying structures and how
data is organised so that commands don't look like magic spells that
solve problems. It helps people build a mental model of how the software
works so that they can solve most problems themselves or atleast ask the
right questions. I actively discourage people from using IDE plugins and
graphical front ends for the training because they hide the details
which I think are important.

I use gitk and some home brew scripts to show how the DAG and objects
get created and their structures but that is purely for illustrative
purposes.

I also mention a few git front ends in passing like magit (which I use)
and tortoise (since many of the trainings I conduct are for Windows
users) at the end of the course so that people are aware of
alternatives. 


-- 
Cordially,
Noufal
http://nibrahim.net.in
--
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


DO YOU NEED BUSINESS AND PERSONAL LOAN

2013-11-19 Thread infor
 Sir/Madam,fast and reliable financial support I offer loan of €5,000 to 
€950,000 to anyone able to repay with interest. For anyone interested email me 
URGENTLY Name,Amount,Duration,Phone and Country,age,sex 
--
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 Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:

 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.

That is probably because the patch is trying to collect a wrong kind
of information, I think. If the approach is to check if each atom is
a color atom, parse_atom() may be the right place, but that is not
necessary.

The only thing you need to know is if you need to emit a reset
that the user did not explicitly ask for at the end, and that is a
property of the format string, which is constant across refs you are
iterating over. You do not even need to know which one of the atom
is of the color kind.

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

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().

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
@@ -23,7 +23,6 @@ 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 {
@@ -94,6 +93,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 need_color_reset_at_eol;
 
 /*
  * Used to parse format string and sort specifiers
@@ -180,13 +180,21 @@ static const char *find_next(const char *cp)
 static int verify_format(const char *format)
 {
const char *cp, *sp;
+   static const char color_reset[] = color:reset;
+
+   need_color_reset_at_eol = 0;
for (cp = format; *cp  (sp = find_next(cp)); ) {
const char *ep = strchr(sp, ')');
+   int at;
+
if (!ep)
return error(malformed format string %s, sp);
/* sp points at %( and ep points at the closing ) */
-   parse_atom(sp + 2, ep);
+   at = parse_atom(sp + 2, ep);
cp = ep + 1;
+
+   if (!memcmp(used_atom[at], color:, 6))
+   need_color_reset_at_eol = !!strcmp(used_atom[at], 
color_reset);
}
return 0;
 }
@@ -644,7 +652,7 @@ static void populate_value(struct refinfo *ref)
int deref = 0;
const char *refname;
const char *formatp;
-   struct branch *branch;
+   struct branch *branch = NULL;
 
if (*name == '*') {
deref = 1;
@@ -669,10 +677,6 @@ 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)) {
@@ -730,7 +734,7 @@ static void populate_value(struct refinfo *ref)
continue;
} else if (!strcmp(formatp, trackshort) 
!prefixcmp(name, upstream)) {
-
+   assert(branch);
stat_tracking_info(branch, num_ours, 
num_theirs);
if (!num_ours  !num_theirs)
v-s = =;
@@ -986,35 +990,28 @@ 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) {
+

Re: Command-line git Vs IDE+plugin?

2013-11-19 Thread Junio C Hamano
Thomas Koch tho...@koch.ro writes:

 I'm a software engineer now with an education as a high school teacher. From 
 a 
 theoretical point of view it's preferable to avoid any abstraction done by a 
 GUI and use commandline Git. Only gitk is useful to have a visual _feedback_ 
 of the actions done on the commandline.

 But also from experience I can tell that without exception everybody whom I 
 teached Git understood it only after being introduced to the basic concepts 
 of 
 Git and how to inspect and operate them on the commandline. Others told me 
 from similar experiences.
 ...
 My collegues meanwhile dumped their graphical Git tool because they learned 
 that they have better control over Git when using it from the commandline.

Interesting.  I think any UI needs to fill three objectives:

 - make common tasks easy;
 - make complex tasks possible; and
 - help users build the right mental model.

As a tool from Linus school, we started from the second and the
third point.  Our UI (i.e. CLI) has long been notorious for lacking
in the first department, and we have worked long and hard to improve
on that front.  While there are more work needed for CLI, your
observation, and a similar experience by Noufal in the thread, hints
me that the available GUI tools have concentrated primarily on the
first point but are still lacking in the rest.

Perhaps I am being naïve, but I would expect that a GUI is a much
better vehicle to help users build the right mental model.  Unlike
CLI, it has a canvas to draw pretty pictures and present the users
what the user is really doing after all.

And I am sure GUI people will eventually realize that potential in
the tools they are building, and the world will become a better place
for both GUI and CLI users ;-).  I found ungit an interesting
experiment that goes into that direction.

Being forever hopeful...
--
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: Command-line git Vs IDE+plugin?

2013-11-19 Thread Keshav Kini
Thomas Koch tho...@koch.ro writes:
 But also from experience I can tell that without exception everybody whom I 
 teached Git understood it only after being introduced to the basic concepts 
 of 
 Git and how to inspect and operate them on the commandline. Others told me 
 from similar experiences.

 Those concepts are:

 - hashes
 - content adressable storage
 - blops being referenced by trees being referenced by commits

A great reference I always point people to for exactly those reasons is
Sitaram Chamarty's git concepts simplified:

http://gitolite.com/gcs/

-Keshav

--
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: [PATCHv2] transport: Catch non positive --depth option value

2013-11-19 Thread Junio C Hamano
Andrés G. Aragoneses kno...@gmail.com writes:

 Instead of simply ignoring the value passed to --depth
 option when it is zero or negative, now it is caught
 and reported.

 This will let people know that they were using the
 option incorrectly (as depth0 should be simply invalid,
 and under the hood depth==0 didn't have any effect).

 Signed-off-by: Andres G. Aragoneses kno...@gmail.com
 Reviewed-by: Duy Nguyen pclo...@gmail.com
 Reviewed-by: Junio C Hamano gits...@pobox.com
 ---
  transport.c | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/transport.c b/transport.c
 index 7202b77..edd63eb 100644
 --- a/transport.c
 +++ b/transport.c
 @@ -483,6 +483,8 @@ static int set_git_option(struct
 git_transport_options *opts,
  opts-depth = strtol(value, end, 0);
  if (*end)
  die(transport: invalid depth option '%s', value);
 +if (opts-depth  1)
 +die(transport: invalid depth option '%s' (non
 positive), value);

transport: depth option '%s' must be positive, perhaps?

  }
  return 0;
  }

Linewrapped and whitespace damaged.
--
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: Command-line git Vs IDE+plugin?

2013-11-19 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Perhaps I am being naïve, but I would expect that a GUI is a much
 better vehicle to help users build the right mental model.

One thing the command-line does well is to give names to concepts
(basically, command names, option names, ...). It's easy to write in a
tutorial or an email run the command 'git foo'. It's less easy to
write click on that red button, on the right of the green one.
Then, it's clear to everyone what commit, rebase, amend  so mean,
regardless of the colors of buttons (that's one of the reasons I use the
english words even when I speak french).

That said, even when I teach the command-line, gitk is a very valuable
tool to explain what the DAG is and how branching works.

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

2013-11-19 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 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?

We already represent information that is for the format string as
existing globals. It means that, if we ever want to make the program
accept and use more than one format string, we can't.  We need one
set of them for each such format string before we can use more than
one.

If you want to solve that problem, complaining by using a subjective
word ugly does not help us much.  The right approach to the
solution would be to first think what each global really means and
decide which ones are per-format properties.  Then turn them into a
proper abstraction by defining a structure to hold the currently
considered format string and these various per format string
properties.

Once you do that, you can optionally make the code pass that single
structure around, and that will remove the global, but I think that
step can wait until we actually find a need 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 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


[PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message

2013-11-19 Thread Jens Lehmann
When using the '-v' option of git commit the diff added to the commit
message temporarily for editing is stripped off after the user exited the
editor by searching for \ndiff --git  and truncating the commmit message
there if it is found. But this approach has two problems: when the commit
message itself contains a line starting with diff --git it will be
truncated there prematurely. And when the diff.submodule setting is set
to log, the diff may start with Submodule hash1..hash2, which will
be left in the commit message while it shouldn't.

Fix that by introducing a special scissor separator line starting with the
comment character '#' followed by a line describing what it is for. The
scissor line - which will not be translated - is used to reliably detect
the start of the diff so it can be chopped off from the commit message, no
matter what the user enters there. Turn a known test failure fixed by this
change into a successful test and add another one for a diff starting with
a submodule log.

Reported-by: Ari Pollak a...@debian.org
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---

Am 18.11.2013 17:01, schrieb Junio C Hamano:
 Jeff King p...@peff.net writes:
 
 I found this hard to parse, I think because of the keeping (why would
 I not keep it?), and because you are talking about lines above and
 below. It is not as accurate to say:

   # -- 8 
   # Everything below this line will be removed.

 because it is technically the line above that is the cutoff. But I think
 the meaning is clear, and it is simpler to parse.

Ok, changed in this version.

 I agree with your rewording suggestion.  It might make it even more
 robust to do something like
 
 const char cut_here[] = # --- cut here --- 8 --- cut here ---;
 
 fprintf(fh, %s\n, cut_here);
 fputs(_(# Everything below this line will be removed\n), fh);
 ...
 p = strstr(cut_here);
 
 i.e. a real marker line that will never be translated, with an
 explanation immediately below that can be translated.

Which is what my last version already did :-)  But it didn't mention
that in the commit message, which it does now).

Also the too strict check for the scissor line is fixed according to
Peff's proposal, it will now handle an empty commit message correctly
too.


 builtin/commit.c  |  6 +++---
 t/t7507-commit-verbose.sh | 15 ++-
 wt-status.c   |  4 
 wt-status.h   |  2 ++
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 6ab4605..b6d6655 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)

/* Truncate the message just before the diff, if any. */
if (verbose) {
-   p = strstr(sb.buf, \ndiff --git );
-   if (p != NULL)
-   strbuf_setlen(sb, p - sb.buf + 1);
+   p = strstr(sb.buf, wt_status_diff_divider);
+   if (p  (p == sb.buf || p[-1] == '\n'))
+   strbuf_setlen(sb, p - sb.buf);
}

if (cleanup_mode != CLEANUP_NONE)
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index da5bd3b..09c1150 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -65,9 +65,22 @@ test_expect_success 'diff in message is retained without -v' 
'
check_message diff
 '

-test_expect_failure 'diff in message is retained with -v' '
+test_expect_success 'diff in message is retained with -v' '
git commit --amend -F diff -v 
check_message diff
 '

+test_expect_success 'submodule log is stripped out too with -v' '
+   git config diff.submodule log 
+   git submodule add ./. sub 
+   git commit -m sub added 
+   (
+   cd sub 
+   echo more file 
+   git commit -a -m submodule commit
+   ) 
+   GIT_EDITOR=cat test_must_fail git commit -a -v 2err 
+   test_i18ngrep Aborting commit due to empty commit message. err
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index b4e44ba..a499bd2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,8 @@
 #include column.h
 #include strbuf.h

+const char wt_status_diff_divider[] = # 
---8---\n;
+
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
GIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */
@@ -791,6 +793,8 @@ static void wt_status_print_verbose(struct wt_status *s)
 */
if (s-fp != stdout)
rev.diffopt.use_color = 0;
+   fprintf(s-fp, wt_status_diff_divider);
+   fprintf(s-fp, _(# Everything below this line will be removed.\n));
run_diff_index(rev, 1);
 }

diff --git a/wt-status.h b/wt-status.h
index 6c29e6f..cd2709f 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -91,6 +91,8 @@ struct 

Re: [PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message

2013-11-19 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 When using the '-v' option of git commit the diff added to the commit
 message temporarily for editing is stripped off after the user exited the
 editor by searching for \ndiff --git  and truncating the commmit message
 there if it is found. But this approach has two problems: when the commit
 message itself contains a line starting with diff --git it will be
 truncated there prematurely. And when the diff.submodule setting is set
 to log, the diff may start with Submodule hash1..hash2, which will
 be left in the commit message while it shouldn't.

 Fix that by introducing a special scissor separator line starting with the
 comment character '#' followed by a line describing what it is for. The
 scissor line - which will not be translated - is used to reliably detect
 the start of the diff so it can be chopped off from the commit message, no
 matter what the user enters there. Turn a known test failure fixed by this
 change into a successful test and add another one for a diff starting with
 a submodule log.

 Reported-by: Ari Pollak a...@debian.org
 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---

 Am 18.11.2013 17:01, schrieb Junio C Hamano:
 Jeff King p...@peff.net writes:
 
 I found this hard to parse, I think because of the keeping (why would
 I not keep it?), and because you are talking about lines above and
 below. It is not as accurate to say:

   # -- 8 
   # Everything below this line will be removed.

 because it is technically the line above that is the cutoff. But I think
 the meaning is clear, and it is simpler to parse.

 Ok, changed in this version.

 I agree with your rewording suggestion.  It might make it even more
 robust to do something like
 
 const char cut_here[] = # --- cut here --- 8 --- cut here ---;
 
 fprintf(fh, %s\n, cut_here);
 fputs(_(# Everything below this line will be removed\n), fh);
 ...
 p = strstr(cut_here);
 
 i.e. a real marker line that will never be translated, with an
 explanation immediately below that can be translated.

 Which is what my last version already did :-)  But it didn't mention
 that in the commit message, which it does now).

 Also the too strict check for the scissor line is fixed according to
 Peff's proposal, it will now handle an empty commit message correctly
 too.

Thanks.

I have a feeling that the translatable message has to say two
things, though.

 - Any junk below that cut marker will be removed;

 - Do not touch the cut marker, or you will mess up the commit log
   message.

Nobody seems to have brought up the latter in the discussion, but I
have a feeling that the users can do almost anything to break us and
it is better to be defensive for the users.

Perhaps phrase it something like this (the message is to be
translated, but not the marker)?

#  8 
# Do not touch the cut-marker line above; everything below
# will be removed.


  builtin/commit.c  |  6 +++---
  t/t7507-commit-verbose.sh | 15 ++-
  wt-status.c   |  4 
  wt-status.h   |  2 ++
  4 files changed, 23 insertions(+), 4 deletions(-)

 diff --git a/builtin/commit.c b/builtin/commit.c
 index 6ab4605..b6d6655 100644
 --- a/builtin/commit.c
 +++ b/builtin/commit.c
 @@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char 
 *prefix)

   /* Truncate the message just before the diff, if any. */
   if (verbose) {
 - p = strstr(sb.buf, \ndiff --git );
 - if (p != NULL)
 - strbuf_setlen(sb, p - sb.buf + 1);
 + p = strstr(sb.buf, wt_status_diff_divider);
 + if (p  (p == sb.buf || p[-1] == '\n'))
 + strbuf_setlen(sb, p - sb.buf);
   }

   if (cleanup_mode != CLEANUP_NONE)
 diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
 index da5bd3b..09c1150 100755
 --- a/t/t7507-commit-verbose.sh
 +++ b/t/t7507-commit-verbose.sh
 @@ -65,9 +65,22 @@ test_expect_success 'diff in message is retained without 
 -v' '
   check_message diff
  '

 -test_expect_failure 'diff in message is retained with -v' '
 +test_expect_success 'diff in message is retained with -v' '
   git commit --amend -F diff -v 
   check_message diff
  '

 +test_expect_success 'submodule log is stripped out too with -v' '
 + git config diff.submodule log 
 + git submodule add ./. sub 
 + git commit -m sub added 
 + (
 + cd sub 
 + echo more file 
 + git commit -a -m submodule commit
 + ) 
 + GIT_EDITOR=cat test_must_fail git commit -a -v 2err 
 + test_i18ngrep Aborting commit due to empty commit message. err
 +'
 +
  test_done
 diff --git a/wt-status.c b/wt-status.c
 index b4e44ba..a499bd2 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -16,6 +16,8 @@
  #include column.h
  #include strbuf.h

 +const 

Re: [PATCH v4 2/2] Rename suffixcmp() to ends_with() and invert its result

2013-11-19 Thread Jonathan Nieder
Christian Couder wrote:

 Now ends_with() returns 1 when the suffix is present and 0 otherwise.

Sounds good.

[...]
 And in vcs-svn/fast_export.c there was already an ends_with()
 function that did the same thing. Let's used the renamed one
 while at it.

Yes, despite the change in signature this shouldn't slow anything
down.  Thanks.

For what it's worth,
Reviewed-by: Jonathan Nieder jrnie...@gmail.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 v2 ] commit -v: strip diffs and submodule shortlogs from the commit message

2013-11-19 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 diff --git a/wt-status.h b/wt-status.h
 index 6c29e6f..cd2709f 100644
 --- a/wt-status.h
 +++ b/wt-status.h
 @@ -91,6 +91,8 @@ struct wt_status_state {
   unsigned char cherry_pick_head_sha1[20];
  };

 +const char wt_status_diff_divider[];

This gives me:

./wt-status.h:94:12: error: array 'wt_status_diff_divider' assumed to have one 
element [-Werror]
cc1: all warnings being treated as errors

which is a bit unfortunate.

Regardless of that, from the API design standpoint, I think it may
be much better not to expose this particular implementation element
(i.e. the array) to the caller, but instead to export a helper
function that takes a pointer to a piece of memory and let callers
ask an I have this line---is it the status cut mark? question.
--
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 ] commit -v: strip diffs and submodule shortlogs from the commit message

2013-11-19 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Jens Lehmann jens.lehm...@web.de writes:

 diff --git a/wt-status.h b/wt-status.h
 index 6c29e6f..cd2709f 100644
 --- a/wt-status.h
 +++ b/wt-status.h
 @@ -91,6 +91,8 @@ struct wt_status_state {
  unsigned char cherry_pick_head_sha1[20];
  };

 +const char wt_status_diff_divider[];

 This gives me:

 ./wt-status.h:94:12: error: array 'wt_status_diff_divider' assumed to have 
 one element [-Werror]
 cc1: all warnings being treated as errors

 which is a bit unfortunate.

 Regardless of that, from the API design standpoint, I think it may
 be much better not to expose this particular implementation element
 (i.e. the array) to the caller, but instead to export a helper
 function that takes a pointer to a piece of memory and let callers
 ask an I have this line---is it the status cut mark? question.

That is, something like this, perhaps.

 builtin/commit.c  |  9 +++--
 t/t7507-commit-verbose.sh | 15 ++-
 wt-status.c   | 15 +++
 wt-status.h   |  1 +
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 6ab4605..fedb45a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1505,7 +1505,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
struct strbuf sb = STRBUF_INIT;
struct strbuf author_ident = STRBUF_INIT;
const char *index_file, *reflog_msg;
-   char *nl, *p;
+   char *nl;
unsigned char sha1[20];
struct ref_lock *ref_lock;
struct commit_list *parents = NULL, **pptr = parents;
@@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
}
 
/* Truncate the message just before the diff, if any. */
-   if (verbose) {
-   p = strstr(sb.buf, \ndiff --git );
-   if (p != NULL)
-   strbuf_setlen(sb, p - sb.buf + 1);
-   }
+   if (verbose)
+   wt_status_truncate_message_at_cut_line(sb);
 
if (cleanup_mode != CLEANUP_NONE)
stripspace(sb, cleanup_mode == CLEANUP_ALL);
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index da5bd3b..09c1150 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -65,9 +65,22 @@ test_expect_success 'diff in message is retained without -v' 
'
check_message diff
 '
 
-test_expect_failure 'diff in message is retained with -v' '
+test_expect_success 'diff in message is retained with -v' '
git commit --amend -F diff -v 
check_message diff
 '
 
+test_expect_success 'submodule log is stripped out too with -v' '
+   git config diff.submodule log 
+   git submodule add ./. sub 
+   git commit -m sub added 
+   (
+   cd sub 
+   echo more file 
+   git commit -a -m submodule commit
+   ) 
+   GIT_EDITOR=cat test_must_fail git commit -a -v 2err 
+   test_i18ngrep Aborting commit due to empty commit message. err
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index b4e44ba..492506a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,9 @@
 #include column.h
 #include strbuf.h
 
+static const char wt_status_cut_line[] =
+#  8 \n;
+
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
GIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */
@@ -767,6 +770,15 @@ conclude:
status_printf_ln(s, GIT_COLOR_NORMAL, );
 }
 
+void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
+{
+   const char *p;
+
+   p = strstr(buf-buf, wt_status_cut_line);
+   if (p  (p == buf-buf || p[-1] == '\n'))
+   strbuf_setlen(buf, p - buf-buf);
+}
+
 static void wt_status_print_verbose(struct wt_status *s)
 {
struct rev_info rev;
@@ -791,6 +803,9 @@ static void wt_status_print_verbose(struct wt_status *s)
 */
if (s-fp != stdout)
rev.diffopt.use_color = 0;
+   fprintf(s-fp, wt_status_cut_line);
+   fprintf(s-fp, _(# Do not touch the line above.\n));
+   fprintf(s-fp, _(# Everything below will be removed.\n));
run_diff_index(rev, 1);
 }
 
diff --git a/wt-status.h b/wt-status.h
index 6c29e6f..30a4812 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -91,6 +91,7 @@ struct wt_status_state {
unsigned char cherry_pick_head_sha1[20];
 };
 
+void wt_status_truncate_message_at_cut_line(struct strbuf *);
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
--
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 ] commit -v: strip diffs and submodule shortlogs from the commit message

2013-11-19 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 Am 18.11.2013 17:01, schrieb Junio C Hamano:
 Jeff King p...@peff.net writes:
 
 I found this hard to parse, I think because of the keeping (why would
 I not keep it?), and because you are talking about lines above and
 below. It is not as accurate to say:

   # -- 8 
   # Everything below this line will be removed.

 because it is technically the line above that is the cutoff. But I think
 the meaning is clear, and it is simpler to parse.

 Ok, changed in this version.

 I agree with your rewording suggestion

 Which is what my last version already did :-)  But it didn't mention
 that in the commit message, which it does now).

Oh, another thing. Does this interact with the core.commentChar in
any way, and if so how?
--
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 ] commit -v: strip diffs and submodule shortlogs from the commit message

2013-11-19 Thread Jens Lehmann
Am 19.11.2013 21:34, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:
 
 Am 18.11.2013 17:01, schrieb Junio C Hamano:
 Jeff King p...@peff.net writes:

 I found this hard to parse, I think because of the keeping (why would
 I not keep it?), and because you are talking about lines above and
 below. It is not as accurate to say:

   # -- 8 
   # Everything below this line will be removed.

 because it is technically the line above that is the cutoff. But I think
 the meaning is clear, and it is simpler to parse.

 Ok, changed in this version.

 I agree with your rewording suggestion

 Which is what my last version already did :-)  But it didn't mention
 that in the commit message, which it does now).
 
 Oh, another thing. Does this interact with the core.commentChar in
 any way, and if so how?

If I understand that setting correctly the hardcoded '#' characters
must be replaced with the core.commentChar when set. Will tackle that
in the next iteration.
--
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 2/2] Rename suffixcmp() to ends_with() and invert its result

2013-11-19 Thread Christian Couder
From: Jonathan Nieder jrnie...@gmail.com
 Christian Couder wrote:
 
 And in vcs-svn/fast_export.c there was already an ends_with()
 function that did the same thing. Let's used the renamed one
 while at it.
 
 Yes, despite the change in signature this shouldn't slow anything
 down.  Thanks.
 
 For what it's worth,
 Reviewed-by: Jonathan Nieder jrnie...@gmail.com

Thanks.

To avoid spamming the list again, I am going to send the following
patches from the 86 patch long series to replace prefixcmp() with
starts_with():

[PATCH v2 00/86] replace prefixcmp() with starts_with()
[PATCH v2 01/86] strbuf: add starts_with() to be used instead of prefixcmp()
[PATCH v2 02/86] diff: replace prefixcmp() with starts_with()
[PATCH v2 08/86] transport*: replace prefixcmp() with starts_with()
[PATCH v2 40/86] environment: replace prefixcmp() with starts_with()
[PATCH v2 86/86] strbuf: remove prefixcmp() as it has been replaced with 
starts_with()

If there are no problems with them, then I will suppose that most of
the patches are ok and probably send them all unless I am asked not
to.

Cheers,
Christian.
--
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 02/86] diff: replace prefixcmp() with starts_with()

2013-11-19 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 diff.c | 56 
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/diff.c b/diff.c
index a04a34d..4b42997 100644
--- a/diff.c
+++ b/diff.c
@@ -235,7 +235,7 @@ int git_diff_basic_config(const char *var, const char 
*value, void *cb)
if (userdiff_config(var, value)  0)
return -1;
 
-   if (!prefixcmp(var, diff.color.) || !prefixcmp(var, color.diff.)) {
+   if (starts_with(var, diff.color.) || starts_with(var, color.diff.)) 
{
int slot = parse_diff_color_slot(var, 11);
if (slot  0)
return 0;
@@ -264,7 +264,7 @@ int git_diff_basic_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
-   if (!prefixcmp(var, submodule.))
+   if (starts_with(var, submodule.))
return parse_submodule_config_option(var, value);
 
return git_default_config(var, value, cb);
@@ -1215,7 +1215,7 @@ static void fn_out_consume(void *priv, char *line, 
unsigned long len)
diff_words_append(line, len,
  ecbdata-diff_words-plus);
return;
-   } else if (!prefixcmp(line, \\ )) {
+   } else if (starts_with(line, \\ )) {
/*
 * Eat the no newline at eof marker as if we
 * saw a + or - line with nothing on it,
@@ -2387,9 +2387,9 @@ static void builtin_diff(const char *name_a,
xdiff_set_find_func(xecfg, pe-pattern, pe-cflags);
if (!diffopts)
;
-   else if (!prefixcmp(diffopts, --unified=))
+   else if (starts_with(diffopts, --unified=))
xecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);
-   else if (!prefixcmp(diffopts, -u))
+   else if (starts_with(diffopts, -u))
xecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);
if (o-word_diff)
init_diff_words_data(ecbdata, o, one, two);
@@ -3388,7 +3388,7 @@ int parse_long_opt(const char *opt, const char **argv,
if (arg[0] != '-' || arg[1] != '-')
return 0;
arg += strlen(--);
-   if (prefixcmp(arg, opt))
+   if (!starts_with(arg, opt))
return 0;
arg += strlen(opt);
if (*arg == '=') { /* sticked form: --option=value */
@@ -3419,7 +3419,7 @@ static int stat_opt(struct diff_options *options, const 
char **av)
 
switch (*arg) {
case '-':
-   if (!prefixcmp(arg, -width)) {
+   if (starts_with(arg, -width)) {
arg += strlen(-width);
if (*arg == '=')
width = strtoul(arg + 1, end, 10);
@@ -3429,7 +3429,7 @@ static int stat_opt(struct diff_options *options, const 
char **av)
width = strtoul(av[1], end, 10);
argcount = 2;
}
-   } else if (!prefixcmp(arg, -name-width)) {
+   } else if (starts_with(arg, -name-width)) {
arg += strlen(-name-width);
if (*arg == '=')
name_width = strtoul(arg + 1, end, 10);
@@ -3439,7 +3439,7 @@ static int stat_opt(struct diff_options *options, const 
char **av)
name_width = strtoul(av[1], end, 10);
argcount = 2;
}
-   } else if (!prefixcmp(arg, -graph-width)) {
+   } else if (starts_with(arg, -graph-width)) {
arg += strlen(-graph-width);
if (*arg == '=')
graph_width = strtoul(arg + 1, end, 10);
@@ -3449,7 +3449,7 @@ static int stat_opt(struct diff_options *options, const 
char **av)
graph_width = strtoul(av[1], end, 10);
argcount = 2;
}
-   } else if (!prefixcmp(arg, -count)) {
+   } else if (starts_with(arg, -count)) {
arg += strlen(-count);
if (*arg == '=')
count = strtoul(arg + 1, end, 10);
@@ -3611,15 +3611,15 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
options-output_format |= DIFF_FORMAT_SHORTSTAT;
else if (!strcmp(arg, -X) || !strcmp(arg, --dirstat))
return parse_dirstat_opt(options, );
-   else if (!prefixcmp(arg, -X))
+   else if (starts_with(arg, -X))
return parse_dirstat_opt(options, arg + 2);
-   else if (!prefixcmp(arg, --dirstat=))
+   else if (starts_with(arg, 

[PATCH v2 00/86] replace prefixcmp() with starts_with()

2013-11-19 Thread Christian Couder
Here is a resend of a big patch series to replace prefixcmp()
with a new starts_with() function.

The changes since the previous version are the following

- the function to replace prefixcmp() is now
  starts_with()
- the mispelling of prefixcmp() in the titles
  of the patches and emails is fixed

The first patch of this series introduces starts_with() and
the last patch removes prefixcmp().

Except a few patches, it's possible to generate the patches
in between using a script like the following: 

===
#!/bin/bash

perl -pi -e 's/!prefixcmp\(/starts_with\(/g' $1
perl -pi -e 's/prefixcmp\(/!starts_with\(/g' $1

git commit -m $1: replace prefixcmp() with starts_with() $1
===

The few special cases are the following ones:

- remote*: replace prefixcmp() with starts_with()
- transport*: replace prefixcmp() with starts_with()
- environment: replace prefixcmp() with starts_with()

In first 2 cases above, I processed a few files at the same
time instead of just one.

In the case of environment, I removed  != 0 after
!starts_with(...) as it is not necessary and makes it
more difficult to understand the logic.

Of course it's possible to squash many of the commits
together if it is prefered.


Christian Couder (86):
  strbuf: add starts_with() to be used instead of prefixcmp()
  diff: replace prefixcmp() with starts_with()
  fast-import: replace prefixcmp() with starts_with()
  remote*: replace prefixcmp() with starts_with()
  daemon: replace prefixcmp() with starts_with()
  pretty: replace prefixcmp() with starts_with()
  revision: replace prefixcmp() with starts_with()
  transport*: replace prefixcmp() with starts_with()
  config: replace prefixcmp() with starts_with()
  sha1_name: replace prefixcmp() with starts_with()
  wt-status: replace prefixcmp() with starts_with()
  upload-pack: replace prefixcmp() with starts_with()
  test-line-buffer: replace prefixcmp() with starts_with()
  parse-options: replace prefixcmp() with starts_with()
  fetch-pack: replace prefixcmp() with starts_with()
  git: replace prefixcmp() with starts_with()
  tag: replace prefixcmp() with starts_with()
  sequencer: replace prefixcmp() with starts_with()
  commit: replace prefixcmp() with starts_with()
  http: replace prefixcmp() with starts_with()
  imap-send: replace prefixcmp() with starts_with()
  help: replace prefixcmp() with starts_with()
  log-tree: replace prefixcmp() with starts_with()
  merge-recursive: replace prefixcmp() with starts_with()
  notes: replace prefixcmp() with starts_with()
  refs: replace prefixcmp() with starts_with()
  setup: replace prefixcmp() with starts_with()
  bisect: replace prefixcmp() with starts_with()
  branch: replace prefixcmp() with starts_with()
  http-push: replace prefixcmp() with starts_with()
  send-pack: replace prefixcmp() with starts_with()
  http-backend: replace prefixcmp() with starts_with()
  notes-utils: replace prefixcmp() with starts_with()
  pkt-line: replace prefixcmp() with starts_with()
  alias: replace prefixcmp() with starts_with()
  attr: replace prefixcmp() with starts_with()
  connect: replace prefixcmp() with starts_with()
  pager: replace prefixcmp() with starts_with()
  convert: replace prefixcmp() with starts_with()
  environment: replace prefixcmp() with starts_with()
  shell: replace prefixcmp() with starts_with()
  pathspec: replace prefixcmp() with starts_with()
  submodule: replace prefixcmp() with starts_with()
  test-string-list: replace prefixcmp() with starts_with()
  builtin/apply: replace prefixcmp() with starts_with()
  builtin/archive: replace prefixcmp() with starts_with()
  builtin/branch: replace prefixcmp() with starts_with()
  builtin/checkout: replace prefixcmp() with starts_with()
  builtin/clean: replace prefixcmp() with starts_with()
  builtin/clone: replace prefixcmp() with starts_with()
  builtin/column: replace prefixcmp() with starts_with()
  builtin/commit: replace prefixcmp() with starts_with()
  builtin/describe: replace prefixcmp() with starts_with()
  builtin/fast-export: replace prefixcmp() with starts_with()
  builtin/fetch-pack: replace prefixcmp() with starts_with()
  builtin/fetch: replace prefixcmp() with starts_with()
  builtin/fmt-merge-msg: replace prefixcmp() with starts_with()
  builtin/for-each-ref: replace prefixcmp() with starts_with()
  builtin/fsck: replace prefixcmp() with starts_with()
  builtin/help: replace prefixcmp() with starts_with()
  builtin/index-pack: replace prefixcmp() with starts_with()
  builtin/init-db: replace prefixcmp() with starts_with()
  builtin/log: replace prefixcmp() with starts_with()
  builtin/ls-remote: replace prefixcmp() with starts_with()
  builtin/mailinfo: replace prefixcmp() with starts_with()
  builtin/merge-recursive: replace prefixcmp() with starts_with()
  builtin/merge: replace prefixcmp() with starts_with()
  builtin/name-rev: replace prefixcmp() with starts_with()
  builtin/notes: replace prefixcmp() with starts_with()
  

[PATCH v2 01/86] strbuf: add starts_with() to be used instead of prefixcmp()

2013-11-19 Thread Christian Couder
prefixcmp() cannot be really used as a comparison function as
it is not antisymmetric:

prefixcmp(foo, foobar)  0
prefixcmp(foobar, foo) == 0

So it is not suitable as a function for passing to qsort.
And in fact it is used nowhere as a comparison function.

So we should replace it with a function that just checks for
equality.

As a first step toward this goal, this patch introduces
starts_with().

Some popular programming languages have functions or methods
called using start and with that are doing what we want.
Therefore it makes sense to use starts_with() as a function
name to replace prefixcmp().

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 git-compat-util.h | 1 +
 strbuf.c  | 9 +
 2 files changed, 10 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 37f0ba0..e441a6b 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -351,6 +351,7 @@ extern void set_error_routine(void (*routine)(const char 
*err, va_list params));
 extern void set_die_is_recursing_routine(int (*routine)(void));
 
 extern int prefixcmp(const char *str, const char *prefix);
+extern int starts_with(const char *str, const char *prefix);
 extern int ends_with(const char *str, const char *suffix);
 
 static inline const char *skip_prefix(const char *str, const char *prefix)
diff --git a/strbuf.c b/strbuf.c
index 2a14fdb..933d998 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -10,6 +10,15 @@ int prefixcmp(const char *str, const char *prefix)
return (unsigned char)*prefix - (unsigned char)*str;
 }
 
+int starts_with(const char *str, const char *prefix)
+{
+   for (; ; str++, prefix++)
+   if (!*prefix)
+   return 1;
+   else if (*str != *prefix)
+   return 0;
+}
+
 int ends_with(const char *str, const char *suffix)
 {
int len = strlen(str), suflen = strlen(suffix);
-- 
1.8.4.1.561.g12affca


--
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 86/86] strbuf: remove prefixcmp() as it has been replaced with starts_with()

2013-11-19 Thread Christian Couder
prefixcmp() is now useless as the previous commit replaced it
everywhere with starts_with(). So let's now remove it.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 git-compat-util.h | 1 -
 strbuf.c  | 9 -
 2 files changed, 10 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index e441a6b..c4c01e7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -350,7 +350,6 @@ extern void set_die_routine(NORETURN_PTR void 
(*routine)(const char *err, va_lis
 extern void set_error_routine(void (*routine)(const char *err, va_list 
params));
 extern void set_die_is_recursing_routine(int (*routine)(void));
 
-extern int prefixcmp(const char *str, const char *prefix);
 extern int starts_with(const char *str, const char *prefix);
 extern int ends_with(const char *str, const char *suffix);
 
diff --git a/strbuf.c b/strbuf.c
index 933d998..ee96dcf 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,15 +1,6 @@
 #include cache.h
 #include refs.h
 
-int prefixcmp(const char *str, const char *prefix)
-{
-   for (; ; str++, prefix++)
-   if (!*prefix)
-   return 0;
-   else if (*str != *prefix)
-   return (unsigned char)*prefix - (unsigned char)*str;
-}
-
 int starts_with(const char *str, const char *prefix)
 {
for (; ; str++, prefix++)
-- 
1.8.4.1.561.g12affca

--
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 08/86] transport*: replace prefixcmp() with starts_with()

2013-11-19 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 transport-helper.c | 16 
 transport.c| 28 ++--
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index b32e2d6..96de26d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -190,7 +190,7 @@ static struct child_process *get_helper(struct transport 
*transport)
data-export = 1;
else if (!strcmp(capname, check-connectivity))
data-check_connectivity = 1;
-   else if (!data-refspecs  !prefixcmp(capname, refspec )) {
+   else if (!data-refspecs  starts_with(capname, refspec )) {
ALLOC_GROW(refspecs,
   refspec_nr + 1,
   refspec_alloc);
@@ -199,17 +199,17 @@ static struct child_process *get_helper(struct transport 
*transport)
data-connect = 1;
} else if (!strcmp(capname, signed-tags)) {
data-signed_tags = 1;
-   } else if (!prefixcmp(capname, export-marks )) {
+   } else if (starts_with(capname, export-marks )) {
struct strbuf arg = STRBUF_INIT;
strbuf_addstr(arg, --export-marks=);
strbuf_addstr(arg, capname + strlen(export-marks ));
data-export_marks = strbuf_detach(arg, NULL);
-   } else if (!prefixcmp(capname, import-marks)) {
+   } else if (starts_with(capname, import-marks)) {
struct strbuf arg = STRBUF_INIT;
strbuf_addstr(arg, --import-marks=);
strbuf_addstr(arg, capname + strlen(import-marks ));
data-import_marks = strbuf_detach(arg, NULL);
-   } else if (!prefixcmp(capname, no-private-update)) {
+   } else if (starts_with(capname, no-private-update)) {
data-no_private_update = 1;
} else if (mandatory) {
die(Unknown mandatory capability %s. This remote 
@@ -310,7 +310,7 @@ static int set_helper_option(struct transport *transport,
 
if (!strcmp(buf.buf, ok))
ret = 0;
-   else if (!prefixcmp(buf.buf, error)) {
+   else if (starts_with(buf.buf, error)) {
ret = -1;
} else if (!strcmp(buf.buf, unsupported))
ret = 1;
@@ -374,7 +374,7 @@ static int fetch_with_fetch(struct transport *transport,
while (1) {
recvline(data, buf);
 
-   if (!prefixcmp(buf.buf, lock )) {
+   if (starts_with(buf.buf, lock )) {
const char *name = buf.buf + 5;
if (transport-pack_lockfile)
warning(%s also locked %s, data-name, name);
@@ -645,10 +645,10 @@ static int push_update_ref_status(struct strbuf *buf,
char *refname, *msg;
int status;
 
-   if (!prefixcmp(buf-buf, ok )) {
+   if (starts_with(buf-buf, ok )) {
status = REF_STATUS_OK;
refname = buf-buf + 3;
-   } else if (!prefixcmp(buf-buf, error )) {
+   } else if (starts_with(buf-buf, error )) {
status = REF_STATUS_REMOTE_REJECT;
refname = buf-buf + 6;
} else
diff --git a/transport.c b/transport.c
index 7202b77..8023956 100644
--- a/transport.c
+++ b/transport.c
@@ -169,13 +169,13 @@ static void set_upstreams(struct transport *transport, 
struct ref *refs,
remotename = ref-name;
tmp = resolve_ref_unsafe(localname, sha, 1, flag);
if (tmp  flag  REF_ISSYMREF 
-   !prefixcmp(tmp, refs/heads/))
+   starts_with(tmp, refs/heads/))
localname = tmp;
 
/* Both source and destination must be local branches. */
-   if (!localname || prefixcmp(localname, refs/heads/))
+   if (!localname || !starts_with(localname, refs/heads/))
continue;
-   if (!remotename || prefixcmp(remotename, refs/heads/))
+   if (!remotename || !starts_with(remotename, refs/heads/))
continue;
 
if (!pretend)
@@ -191,7 +191,7 @@ static void set_upstreams(struct transport *transport, 
struct ref *refs,
 
 static const char *rsync_url(const char *url)
 {
-   return prefixcmp(url, rsync://) ? skip_prefix(url, rsync:) : url;
+   return !starts_with(url, rsync://) ? skip_prefix(url, rsync:) : url;
 }
 
 static struct ref *get_refs_via_rsync(struct transport *transport, int 
for_push)
@@ -296,8 +296,8 @@ static int write_one_ref(const char *name, const unsigned 
char *sha1,
FILE *f;
 
/* when called via for_each_ref(), flags is non-zero */
-

Suggestion for git reference page

2013-11-19 Thread Jim Garrison
The master reference TOC page at http://git-scm.com/docs links to all the 
associated command reference pages, except it seems to be missing a link for 
gitrevisions(7) (http://git-scm.com/docs/gitrevisions.html).

I've never submitted a patch and thought I would learn how... except the 
website source doesn't seem to be in the git source tree.

--
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 2/2] Rename suffixcmp() to ends_with() and invert its result

2013-11-19 Thread Antoine Pelisse
On Tue, Nov 19, 2013 at 10:04 PM, Christian Couder
chrisc...@tuxfamily.org wrote:
 To avoid spamming the list again, I am going to send the following
 patches from the 86 patch long series to replace prefixcmp() with
 starts_with():

 [PATCH v2 00/86] replace prefixcmp() with starts_with()
 [PATCH v2 01/86] strbuf: add starts_with() to be used instead of prefixcmp()
 [PATCH v2 02/86] diff: replace prefixcmp() with starts_with()
 [PATCH v2 08/86] transport*: replace prefixcmp() with starts_with()
 [PATCH v2 40/86] environment: replace prefixcmp() with starts_with()
 [PATCH v2 86/86] strbuf: remove prefixcmp() as it has been replaced with 
 starts_with()

 If there are no problems with them, then I will suppose that most of
 the patches are ok and probably send them all unless I am asked not
 to.

I'm not exactly sure I understand the point of not squashing all those
patches together ?
It's not like one is going without the others, or that the commit
message provides some new information (except for the name of the
file, but that is not very relevant either). The downside is that it's
_many_ messages to bypass when reading mails from small-screen devices
:-)
--
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 00/86] replace prefixcmp() with has_prefix()

2013-11-19 Thread Junio C Hamano
Christian Couder christian.cou...@gmail.com writes:

 There is also a new version of my 86 patch long series to replace
 prefixcmp() with starts_with() that I am ready to send, but I hesitate
 to spam the whole list :-)
 I can put it somewhere like GitHub where people can see everything and
 perhaps send only a few patches to the list, including the first and
 the last.
 @Junio, how would you like me to proceed?

Let's hold this off for now. The other half of this series is
already in 'next' and I do not want to bother with the revert,
rebranch and remerge dance this close to the 1.8.5 final.


--
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: Suggestion for git reference page

2013-11-19 Thread Thomas Rast
Jim Garrison jim.garri...@nwea.org writes:

 The master reference TOC page at http://git-scm.com/docs links to all
 the associated command reference pages, except it seems to be missing
 a link for gitrevisions(7)
 (http://git-scm.com/docs/gitrevisions.html).

 I've never submitted a patch and thought I would learn how... except
 the website source doesn't seem to be in the git source tree.

It's in this repo:

  https://github.com/github/gitscm-next.git

Judging from a quick look, the page you linked to is generated from

  app/views/doc/index.html.haml

Note that they work by pull requests, not patches, i.e. you should push
your changes to a github fork of gitscm-next and send a pull request.

-- 
Thomas Rast
t...@thomasrast.ch
--
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: Suggestion for git reference page

2013-11-19 Thread Jonathan Nieder
Hi,

Jim Garrison wrote:

 The master reference TOC page at http://git-scm.com/docs links to
 all the associated command reference pages, except it seems to be
 missing a link for gitrevisions(7)
 (http://git-scm.com/docs/gitrevisions.html).

 I've never submitted a patch and thought I would learn how... except
 the website source doesn't seem to be in the git source tree.

You might like https://github.com/github/gitscm-next, file
lib/constants.rb.  (It's the target of the 'hosted on GitHub' link at
the bottom of each page, which I admit can be hard to find.)

Hope that helps,
Jonathan
--
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 options '--whitespace=fix' and '--keep-empty' are incompatible

2013-11-19 Thread Nathan Collins
Bug
===

The command

  git rebase --whitespace=fix --keep-empty commit

does not fix whitespace in the rebased commits.

Example
===

Set up a repo with a whitespace error commit and an empty commit:

  git init rebase-bug.git
  cd rebase-bug.git
  touch foo
  git add foo
  touch foo
  git add foo
  git commit -m Empty foo
  echo xxx foo
  git add foo
  git commit -m Trailing whitespace foo
  git commit --allow-empty -m Empty commit

Now we have the following commits (adding '$' at EOL to make trailing
whitespace clear):

  git log --oneline --patch | sed -re 's/$/$/'

  d383707 Empty commit$
  4b71cd0 Trailing whitespace foo$
  diff --git foo foo$
  index e69de29..272a831 100644$
  --- foo$
  +++ foo$
  @@ -0,0 +1 @@$
  +xxx   $
  26d51d6 Empty foo$
  diff --git foo foo$
  new file mode 100644$
  index 000..e69de29$

Make a backup:

  cp -r ../rebase-bug.git{,.backup}

We can now fix the whitespace with 'git rebase --whitespace=fix', but
this drops the empty commit:

  git rebase --whitespace=fix HEAD~2

  Current branch master is up to date, rebase forced.
  First, rewinding head to replay your work on top of it...
  Applying: Trailing whitespace foo

  git log --oneline --patch | sed -re 's/$/$/'

  2f6f66d Trailing whitespace foo$
  diff --git foo foo$
  index e69de29..d6459e0 100644$
  --- foo$
  +++ foo$
  @@ -0,0 +1 @@$
  +xxx$
  26d51d6 Empty foo$
  diff --git foo foo$
  new file mode 100644$
  index 000..e69de29$

If we add '--keep-empty', then we keep the empty commit, but the
whitespace is not fixed:

  cd ../rebase-bug.git.backup
  git rebase --whitespace=fix --keep-empty HEAD~2

  Current branch master is up to date, rebase forced.
  First, rewinding head to replay your work on top of it...
  [detached HEAD a48c4c8] Trailing whitespace foo
   1 file changed, 1 insertion(+)
  [detached HEAD 8a15ca4] Empty commit

  git log --oneline --patch | sed -re 's/$/$/'

  f852c53 Empty commit$
  f8c3626 Trailing whitespace foo$
  diff --git foo foo$
  index e69de29..272a831 100644$
  --- foo$
  +++ foo$
  @@ -0,0 +1 @@$
  +xxx   $
  26d51d6 Empty foo$
  diff --git foo foo$
  new file mode 100644$
  index 000..e69de29$

Git version
===

I'm using git version 1.8.2.3.

Motivation
==

http://stackoverflow.com/a/15398512/470844

I wanted a Git alias to remove whitespace errors from the index and
tree. I tried this:

  fixws-global-tree-and-index = !\
git commit --allow-empty -m FIXWS_SAVE_INDEX  \
git add -u :/  \
git commit --allow-empty -m FIXWS_SAVE_TREE  \
git rebase --whitespace=fix --keep-empty HEAD~2  \
git reset HEAD~  \
git reset --soft HEAD~

But, it does not work, because of the bug illustrated above.  So,
instead, I have this:

  fixws-global-tree-and-index = !\
if (! git diff-files --quiet .)  \
   (! git diff-index --quiet --cached HEAD) ; then \
  git commit -m FIXWS_SAVE_INDEX  \
  git add -u :/  \
  git commit -m FIXWS_SAVE_TREE  \
  git rebase --whitespace=fix HEAD~2  \
  git reset HEAD~  \
  git reset --soft HEAD~ ; \
elif (! git diff-files --quiet .) ; then \
  git add -u :/  \
  git commit -m FIXWS_SAVE_TREE  \
  git rebase --whitespace=fix HEAD~  \
  git reset HEAD~ ; \
elif (! git diff-index --quiet --cached HEAD) ; then \
  git commit -m FIXWS_SAVE_INDEX  \
  git rebase --whitespace=fix HEAD~  \
  git reset --soft HEAD~ ; \
fi

I.e., I calculate which commits are non-empty and only make those
commits.

Cheers,

-nathan
--
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] Fix typesetting in Bugs section of 'git-rebase' man page (web version)

2013-11-19 Thread Jason St. John
Documentation/git-rebase.txt: add a blank line after the two AsciiDoc
listing blocks

Without these blank lines, AsciiDoc thinks the opening - is a
section heading and typesets the word to as such, which causes
cascading formatting/typesetting issues until the end of the document.

Signed-off-by: Jason St. John jstj...@purdue.edu
---
You can see the carnage here:
http://git-scm.com/docs/git-rebase#_bugs

This fixes GitHub issue github/gitscm-next#281
https://github.com/github/gitscm-next/issues/281


 Documentation/git-rebase.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 94e07fd..88d0afb 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -743,9 +743,11 @@ For example, an attempt to rearrange
 1 --- 2 --- 3 --- 4 --- 5
 
 to
+
 
 1 --- 2 --- 4 --- 3 --- 5
 
+
 by moving the pick 4 line will result in the following history:
 
3
-- 
1.8.4.2

--
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] Fix typesetting in Bugs section of 'git-rebase' man page (web version)

2013-11-19 Thread Jonathan Nieder
Hi,

Jason St. John wrote:

 Documentation/git-rebase.txt: add a blank line after the two AsciiDoc
 listing blocks

I'd leave out the above two description lines, since they're redundant
next to the patch text.

 Without these blank lines, AsciiDoc thinks the opening - is a
 section heading and typesets the word to as such, which causes
 cascading formatting/typesetting issues until the end of the document.

Interesting.  Alas, I can't reproduce this.

Are you sure it is AsciiDoc that misinterprets the document, and not,
e.g., the predecessor of asciidoctor in gitscm-next?

Do

  Documentation/git-check-attr.txt
  Documentation/git-commit.txt
  Documentation/git-cvsserver.txt
  Documentation/git-p4.txt
  Documentation/git-svn.txt
  Documentation/gitcli.txt
  Documentation/gitweb.txt
  Documentation/mailmap.txt
  
avoid this problem?

Thanks and hope that helps,
Jonathan
--
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] Fix typesetting in Bugs section of 'git-rebase' man page (web version)

2013-11-19 Thread Jason St. John
On Tue, Nov 19, 2013 at 7:31 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi,

 Jason St. John wrote:

 Documentation/git-rebase.txt: add a blank line after the two AsciiDoc
 listing blocks

 I'd leave out the above two description lines, since they're redundant
 next to the patch text.


I included that because SubmittingPatches says to do so.

 Without these blank lines, AsciiDoc thinks the opening - is a
 section heading and typesets the word to as such, which causes
 cascading formatting/typesetting issues until the end of the document.

 Interesting.  Alas, I can't reproduce this.

 Are you sure it is AsciiDoc that misinterprets the document, and not,
 e.g., the predecessor of asciidoctor in gitscm-next?


I regrettably must admit that I didn't test this before submitting, so
I had presumed that it was AsciiDoc itself.

 Do

   Documentation/git-check-attr.txt
   Documentation/git-commit.txt
   Documentation/git-cvsserver.txt
   Documentation/git-p4.txt
   Documentation/git-svn.txt
   Documentation/gitcli.txt
   Documentation/gitweb.txt
   Documentation/mailmap.txt

 avoid this problem?

 Thanks and hope that helps,
 Jonathan

All of the files you referenced appear to render okay except for
git-svn.txt and gitweb.txt.

Documentation/git-svn.txt:
* Commands | fetch | --ignore-paths
* Commands | dcommit | --commit-url
* Commands | dcommit | --mergeinfo
* Commands | reset | --parent
* Caveats (third paragraph)
See http://git-scm.com/docs/git-svn#_commands and
http://git-scm.com/docs/git-svn#_caveats

Documentation/gitweb.txt:
* This cascades from the virtual host config file example until the
Bugs section.
See: 
http://git-scm.com/docs/gitweb#_webserver_configuration_with_multiple_projects'_root

Based on this and the source files, I suspect the problem lies with,
for example, the predecessor of asciidoctor in gitscm-next.

How do you recommend I proceed? Should I resubmit with just a revised
commit message? Should I incorporate this into a larger patch set that
should fix all of the errors in git-rebase.txt, git-svn.txt, and
gitweb.txt?

Thanks,
Jason
--
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] Fix typesetting in Bugs section of 'git-rebase' man page (web version)

2013-11-19 Thread Jonathan Nieder
Jason St. John wrote:
 On Tue, Nov 19, 2013 at 7:31 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Jason St. John wrote:

 Documentation/git-rebase.txt: add a blank line after the two AsciiDoc
 listing blocks

 I'd leave out the above two description lines, since they're redundant
 next to the patch text.

 I included that because SubmittingPatches says to do so.

Thanks for explaining.  Can you point me to which part of
SubmittingPatches said to include that kind of thing?

[...]
 How do you recommend I proceed? Should I resubmit with just a revised
 commit message? Should I incorporate this into a larger patch set that
 should fix all of the errors in git-rebase.txt, git-svn.txt, and
 gitweb.txt?

Thanks for checking.  You have a few choices.

 a) Check if asciidoctor reproduces the problem, and if so, report it
as a compatibility bug.  Ask the gitscm-next maintainers to get
their copy of asciidoctor up to date.

 b) Come up with a simple rule about how these  blocks should be
formatted, and apply it consistently to the documentation in
Documentation/, with a commit message explaining the story so
future contributors know to continue to follow it.

 c) Resend the same patch that just fixes git-rebase.txt.  Include a
Reported-by line to credit the person who originally caught the
error.  Explain what's going on in the commit message and that
there are other instances of the problem that this patch doesn't
yet fix.

I prefer (b), since it would solve the problem more completely and
means future readers wouldn't be confused about which style to use,
but other possibilities (e.g., some combination of the options listed
above, or something else entirely) could work as well.

Thanks again for tracking this down, and hope that helps,
Jonathan
--
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] diff: restrict pathspec limitations to diff b/f case only

2013-11-19 Thread Nguyễn Thái Ngọc Duy
builtin_diff_b_f() needs a path, not pathspec. Other modes in diff
can deal with pathspec just fine. But because of the current
GUARD_PATHSPEC() location, other modes also reject :(glob) and
:(icase).

Move GUARD_PATHSPEC(), and the path assignment statement, which is
the reason of this GUARD_PATHSPEC(), inside builtin_diff_b_f().

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/diff.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..fe0cc7f 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -64,15 +64,18 @@ static void stuff_change(struct diff_options *opt,
 
 static int builtin_diff_b_f(struct rev_info *revs,
int argc, const char **argv,
-   struct blobinfo *blob,
-   const char *path)
+   struct blobinfo *blob)
 {
/* Blob vs file in the working tree*/
struct stat st;
+   const char *path;
 
if (argc  1)
usage(builtin_diff_usage);
 
+   GUARD_PATHSPEC(revs-prune_data, PATHSPEC_FROMTOP | PATHSPEC_LITERAL);
+   path = revs-prune_data.items[0].match;
+
if (lstat(path, st))
die_errno(_(failed to stat '%s'), path);
if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
@@ -255,7 +258,6 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
struct rev_info rev;
struct object_array ent = OBJECT_ARRAY_INIT;
int blobs = 0, paths = 0;
-   const char *path = NULL;
struct blobinfo blob[2];
int nongit;
int result = 0;
@@ -366,13 +368,8 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
die(_(unhandled object '%s' given.), name);
}
}
-   if (rev.prune_data.nr) {
-   /* builtin_diff_b_f() */
-   GUARD_PATHSPEC(rev.prune_data, PATHSPEC_FROMTOP | 
PATHSPEC_LITERAL);
-   if (!path)
-   path = rev.prune_data.items[0].match;
+   if (rev.prune_data.nr)
paths += rev.prune_data.nr;
-   }
 
/*
 * Now, do the arguments look reasonable?
@@ -385,7 +382,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
case 1:
if (paths != 1)
usage(builtin_diff_usage);
-   result = builtin_diff_b_f(rev, argc, argv, blob, path);
+   result = builtin_diff_b_f(rev, argc, argv, blob);
break;
case 2:
if (paths)
-- 
1.8.2.82.gc24b958

--
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/gitcli.txt: fix double quotes

2013-11-19 Thread Jason St. John
Replace double quotes around literal examples with backticks

Signed-off-by: Jason St. John jstj...@purdue.edu
---
 Documentation/gitcli.txt | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
index 3146413..41bed29 100644
--- a/Documentation/gitcli.txt
+++ b/Documentation/gitcli.txt
@@ -83,12 +83,12 @@ scripting Git:
`git log -1 HEAD` but write `git log -1 HEAD --`; the former will not work
if you happen to have a file called `HEAD` in the work tree.
 
- * many commands allow a long option --option to be abbreviated
+ * many commands allow a long option `--option` to be abbreviated
only to their unique prefix (e.g. if there is no other option
-   whose name begins with opt, you may be able to spell --opt to
-   invoke the --option flag), but you should fully spell them out
+   whose name begins with `opt`, you may be able to spell `--opt` to
+   invoke the `--option` flag), but you should fully spell them out
when writing your scripts; later versions of Git may introduce a
-   new option whose name shares the same prefix, e.g. --optimize,
+   new option whose name shares the same prefix, e.g. `--optimize`,
to make a short prefix that used to be unique no longer unique.
 
 
@@ -149,7 +149,7 @@ prefix of a long option as if it is fully spelled out, but 
use this
 with a caution.  For example, `git commit --amen` behaves as if you
 typed `git commit --amend`, but that is true only until a later version
 of Git introduces another option that shares the same prefix,
-e.g `git commit --amenity option.
+e.g. `git commit --amenity` option.
 
 
 Separating argument from the option
-- 
1.8.4.2

--
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] Support pathspec magic :(exclude) and its short form :-

2013-11-19 Thread Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 This is yet another stab at the negative pathspec thing. It's not
 ready yet (there are a few XXXs) but I could use some feedback
 regarding the interface, or the behavior. It looks better this time
 now that pathspec magic is supported (or maybe I'm just biased).

 For :(glob) or :(icase) you're more likely to enable it for all
 pathspec, i.e. --glob-pathspecs. But I expect :(exclude) to be typed
 more often (it does not make sense to add --exclude-pathspecs to
 exclude everything), which is why I add the short form for it.

 We don't have many options that say negative in short form.
 Either '!', '-' or '~'. '!' is already used for bash history expansion.
 ~ looks more like $HOME expansion. Which left me '-'.

 Documentation/glossary-content.txt |  5 
 builtin/add.c  |  5 +++-
 dir.c  | 50 +++-
 pathspec.c |  9 ++-
 pathspec.h |  4 ++-
 tree-walk.c| 52 +++---
 6 files changed, 112 insertions(+), 13 deletions(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index e470661..f7d7d8c 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -377,6 +377,11 @@ full pathname may have special meaning:
  - Other consecutive asterisks are considered invalid.
 +
 Glob magic is incompatible with literal magic.
+
+exclude `-`;;
+   After a path matches any non-exclude pathspec, it will be run
+   through all exclude pathspec. If it matches, the path is
+   ignored.
 --
 +
 Currently only the slash `/` is recognized as the magic signature,
diff --git a/builtin/add.c b/builtin/add.c
index 226f758..0df73ae 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -540,10 +540,13 @@ int cmd_add(int argc, const char **argv, const char 
*prefix)
   PATHSPEC_FROMTOP |
   PATHSPEC_LITERAL |
   PATHSPEC_GLOB |
-  PATHSPEC_ICASE);
+  PATHSPEC_ICASE |
+  PATHSPEC_EXCLUDE);
 
for (i = 0; i  pathspec.nr; i++) {
const char *path = pathspec.items[i].match;
+   if (pathspec.items[i].magic  PATHSPEC_EXCLUDE)
+   continue;
if (!seen[i] 
((pathspec.items[i].magic 
  (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
diff --git a/dir.c b/dir.c
index 23b6de4..e2df82f 100644
--- a/dir.c
+++ b/dir.c
@@ -126,10 +126,13 @@ static size_t common_prefix_len(const struct pathspec 
*pathspec)
   PATHSPEC_MAXDEPTH |
   PATHSPEC_LITERAL |
   PATHSPEC_GLOB |
-  PATHSPEC_ICASE);
+  PATHSPEC_ICASE |
+  PATHSPEC_EXCLUDE);
 
for (n = 0; n  pathspec-nr; n++) {
size_t i = 0, len = 0, item_len;
+   if (pathspec-items[n].magic  PATHSPEC_EXCLUDE)
+   continue;
if (pathspec-items[n].magic  PATHSPEC_ICASE)
item_len = pathspec-items[n].prefix;
else
@@ -279,9 +282,10 @@ static int match_pathspec_item(const struct pathspec_item 
*item, int prefix,
  * pathspec did not match any names, which could indicate that the
  * user mistyped the nth pathspec.
  */
-int match_pathspec_depth(const struct pathspec *ps,
-const char *name, int namelen,
-int prefix, char *seen)
+static int match_pathspec_depth_1(const struct pathspec *ps,
+ const char *name, int namelen,
+ int prefix, char *seen,
+ int exclude)
 {
int i, retval = 0;
 
@@ -290,7 +294,8 @@ int match_pathspec_depth(const struct pathspec *ps,
   PATHSPEC_MAXDEPTH |
   PATHSPEC_LITERAL |
   PATHSPEC_GLOB |
-  PATHSPEC_ICASE);
+  PATHSPEC_ICASE |
+  PATHSPEC_EXCLUDE);
 
if (!ps-nr) {
if (!ps-recursive ||
@@ -309,6 +314,11 @@ int match_pathspec_depth(const struct pathspec *ps,
 
for (i = ps-nr - 1; i = 0; i--) {
int how;
+
+   if ((!excludeps-items[i].magic  PATHSPEC_EXCLUDE) ||
+   ( exclude  !(ps-items[i].magic  PATHSPEC_EXCLUDE)))
+   continue;
+
if (seen  seen[i] == MATCHED_EXACTLY)
continue;
how = match_pathspec_item(ps-items+i, prefix, name, namelen);
@@ -327,6 +337,16 @@ int match_pathspec_depth(const struct pathspec *ps,
   

defaulting git stash to --keep-index

2013-11-19 Thread Tim Chase
Having lost add -p work enough times when stashing, I finally
dug into the docs to see how to prevent it, discovering that
--keep-index does exactly what I want.  However, now I have trouble
remembering to add the --keep-index until after I've shot myself in
the foot.  How do I go about getting git stash to default to
--keep-index?  I've played around a little with aliases, but
haven't found the right incantation.

The existence of --no-keep-index suggests there's some way to make
--keep-index the default, but I'm missing it.

Thanks,

-tkc



--
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: defaulting git stash to --keep-index

2013-11-19 Thread Nathan Collins
On Tue, Nov 19, 2013 at 5:50 PM, Tim Chase g...@tim.thechases.com wrote:
 Having lost add -p work enough times when stashing, I finally
 dug into the docs to see how to prevent it, discovering that
 --keep-index does exactly what I want.

Note that 'git stash (pop | apply) --index' will reinstate the index
as it was at stash time, regardless of whether '--keep-index' was used
to create the stash.  In other words, your index is not lost when
you stash.

Also note: when you 'git stash --keep-index', although your index
remains intact, the changes in the your index still become part of the
stash.  Hence, any changes to the indexed portion of your files after
a stash usually result in a conflict on subsequent 'git stash pop'.
This confuses me quite a lot, since I'd expect a main use case of 'git
stash --keep-index' to be fixing a up a commit, but then any fixes
cause a conflict :P

Cheers,

-nathan
--
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: defaulting git stash to --keep-index

2013-11-19 Thread Jason St. John
On Tue, Nov 19, 2013 at 8:50 PM, Tim Chase g...@tim.thechases.com wrote:
 Having lost add -p work enough times when stashing, I finally
 dug into the docs to see how to prevent it, discovering that
 --keep-index does exactly what I want.  However, now I have trouble
 remembering to add the --keep-index until after I've shot myself in
 the foot.  How do I go about getting git stash to default to
 --keep-index?  I've played around a little with aliases, but
 haven't found the right incantation.

 The existence of --no-keep-index suggests there's some way to make
 --keep-index the default, but I'm missing it.

 Thanks,

 -tkc



 --
 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-config(1) states that you cannot hide existing Git commands. In
other words, aliasing git stash to git stash --keep-index is not
possible. You could try playing around with the exclamation point
version (e.g. `git config alias.stash = !git stash --keep-index`),
but I suspect that Git will still not allow you to hide the existing
command.

If you are willing to switch to a new command, you can fix this
easily. For example, either of these would do what you want whenever
you run `git stsh` (note the missing a in stash) or `git stashki`:
`git config alias.stsh = stash --keep-index`
`git config alias.stashki = stash --keep-index`

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


Bizarre git merge behaviour

2013-11-19 Thread Matthew Cengia
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

The other day I was merging a feature branch (shown below as
origin/22869-new-kernel) into my staging release (shown below as 'wtf'),
and it *reverted* a bucket-load of changes I'd made on other branches
which had been merged into staging before. I can't for the life of me
work out why this happened, and would appreciate any ideas. Output below
shows symptoms:


  mattcen@sonar:prisonpc(wtf)$ git log --numstat --oneline 
origin/22869-new-kernel ^wtf | cat
  a8879c2 Oops, name .config consistently with deb.
  27970   client/kernel/config-3.5.7.20-1
  0   2797client/kernel/config-3.5.7.21-1
  4c34686 Further lock down the inmate kernel.
  366 1094client/kernel/config-3.5.7.21-1
  -   -   client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb
  -   -   client/kernel/linux-image-3.5.7.20_3.5.7.20-1_i386.deb
  -   -   client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb
  6   7   client/scm/50-kernel-prisoner.scm
  4a8acd0 Add build notes about how we generated these kernels.
  37  0   client/kernel/README
  e96adfe Merge remote-tracking branch 'origin/N-multicast-kernel' into 
22869-new-kernel
  4b8afeb Merge commit '8ef01f2' into 22869-new-kernel
  fc4a5e6 Allow staff machines to receive IPTV stuff (per russm).
  1   0   client/scm/50-staff.scm
  117e646 Merge remote branch 'origin/staging' into N-multicast-kernel
  deb3b38 allow IGMP reports out through iptables
  1   0   client/scm/20-security.scm
  8ef01f2 Enable multicast in inmate kernel.
  0   3034client/kernel/config-2.6.32.46+drm33.20
  30360   client/kernel/config-2.6.32.61+drm33.26
  -   -   
client/kernel/linux-firmware-image_2.6.32.46+drm33.20-1_all.deb
  -   -   
client/kernel/linux-firmware-image_2.6.32.61+drm33.26-1_all.deb
  -   -   
client/kernel/linux-image-2.6.32.46+drm33.20_2.6.32.46+drm33.20-1_i386.deb
  -   -   
client/kernel/linux-image-2.6.32.61+drm33.26_2.6.32.61+drm33.26-1_i386.deb
  11  11  client/scm/50-prisoner.scm
  mattcen@sonar:prisonpc(wtf)$ git merge origin/22869-new-kernel
  Auto-merging tca/parts.d/pxelinux-cfg
  Auto-merging pre-install.sh
  Removing ppcadm/templates/wwwfilter.tpl
  Removing ppcadm/templates/streaming_media.tpl
  Removing ppcadm/templates/disc_summary.tpl
  Removing ppcadm/templates/disc_access.tpl
  Auto-merging ppcadm/modules/quarantine.py
  CONFLICT (content): Merge conflict in ppcadm/modules/quarantine.py
  Auto-merging ppcadm/modules/media.py
  CONFLICT (content): Merge conflict in ppcadm/modules/media.py
  CONFLICT (modify/delete): ppcadm/modules/emailstats.py deleted in 
origin/22869-new-kernel and modified in HEAD. Version HEAD of 
ppcadm/modules/emailstats.py left in tree.
  Removing generate-mail-stats
  Removing doc/adminguide/images/streaming_media-screen.png
  Auto-merging debian/rules
  Auto-merging debian/prisonpc-internet.install
  CONFLICT (content): Merge conflict in debian/prisonpc-internet.install
  Auto-merging debian/prisonpc-core.cron.d
  Auto-merging debian/NEWS
  CONFLICT (content): Merge conflict in debian/NEWS
  Auto-merging client/scm/50-kernel-prisoner.scm
  CONFLICT (content): Merge conflict in client/scm/50-kernel-prisoner.scm
  Removing client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb
  Removing client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb
  Auto-merging client/kernel/config-3.5.7.20-1
  Resolved 'client/scm/50-kernel-prisoner.scm' using previous resolution.
  Resolved 'debian/NEWS' using previous resolution.
  Resolved 'debian/prisonpc-internet.install' using previous resolution.
  Resolved 'ppcadm/modules/media.py' using previous resolution.
  Resolved 'ppcadm/modules/quarantine.py' using previous resolution.
  Automatic merge failed; fix conflicts and then commit the result.
  mattcen@sonar:prisonpc(wtf)$ git status --porcelain
  M  client/kernel/README
  R  client/kernel/config-3.5.7.21-1 - client/kernel/config-3.5.7.20-1
  D  client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb
  A  client/kernel/linux-image-3.5.7.20_3.5.7.20-1_i386.deb
  D  client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb
  M  client/scm/20-security.scm
  M  client/scm/30-apps.scm
  UU client/scm/50-kernel-prisoner.scm
  M  client/scm/50-staff.scm
  UU debian/NEWS
  M  debian/changelog
  M  debian/prisonpc-core.cron.d
  UU debian/prisonpc-internet.install
  M  debian/rules
  A  doc/adminguide/images/streaming_media-screen.jpg
  D  doc/adminguide/images/streaming_media-screen.png
  M  doc/adminguide/images/web_filtering.png
  M  doc/adminguide/streaming_media.rst
  M  doc/adminguide/system-status.rst
  M  doc/adminguide/web_filtering.rst
  M  dovecot/dovecot-staff.conf
  M  eric/apps/complain.py
  M  eric/apps/links.py
  M  eric/apps/media.py
  M  eric/apps/whitelist.py
  M  eric/eric
  M  eric/eric_cfg.py
  D  generate-mail-stats
  M  ppcadm/modules/disc_access.py
  UD 

Re: [PATCH] Fix typesetting in Bugs section of 'git-rebase' man page (web version)

2013-11-19 Thread Junio C Hamano
Jason St. John jstj...@purdue.edu writes:

 Documentation/git-rebase.txt: add a blank line after the two AsciiDoc
 listing blocks

That looks funnily formatted, out of place and redundant.

 Without these blank lines, AsciiDoc thinks the opening - is a
 section heading and typesets the word to as such, which causes
 cascading formatting/typesetting issues until the end of the document.


 Signed-off-by: Jason St. John jstj...@purdue.edu
 ---
 You can see the carnage here:
 http://git-scm.com/docs/git-rebase#_bugs

 This fixes GitHub issue github/gitscm-next#281

Hmph. https://git-htmldocs.googlecode.com/git/git.html has HTML
documentation pages I preformat, but as far as I can see, the bugs
section of git-rebase(1) does not have such a carnage.

Perhaps git-scm.com uses some buggy formatter?
--
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: Bizarre git merge behaviour

2013-11-19 Thread Johannes Sixt
Am 11/20/2013 4:49, schrieb Matthew Cengia:
 The other day I was merging a feature branch (shown below as
 origin/22869-new-kernel) into my staging release (shown below as 'wtf'),
 and it *reverted* a bucket-load of changes I'd made on other branches
 which had been merged into staging before. I can't for the life of me
 work out why this happened, and would appreciate any ideas. Output below
 shows symptoms:

Not really. It's impossible to tell what's wrong if you

- show only ..topic
- but not topic..
- and you keep secret which of the changes is unexpected.

Perhaps you did the following:

- On one branch, you made a change A and then another change B that reverts A.
- On the other branch, you also made the same changes as A, but there is
no equivalent of B.

In this case, it is expected that the changes made by B (i.e. the reversal
of change A) are *NOT* in the merge, i.e., the changes of A survive. It
looks as if the merge reverted change B.

If that is not the case, it could be helpful that you pick one of the
files that contains an unexpected change and post the output of

git log --left-right --oneline wtf...origin/22869-new-kernel \
file/with/changes | cut -c1-70

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