* Re: [PATCH v3 2/3] config: Introduce diff.algorithm variable
From: Junio C Hamano @ 2013-01-16 17:42 UTC (permalink / raw)
To: Michal Privoznik; +Cc: git, trast, peff
In-Reply-To: <4e2aacd5bbf005f0e372589bf423a8cbd776bc6d.1358322212.git.mprivozn@redhat.com>
Will replace the one in 'pu' with this round. Looking good.
Thanks.
^ permalink raw reply
* Re: [PATCH v6 0/8] push: update remote tags only with force
From: Jeff King @ 2013-01-16 17:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Max Horn, Chris Rorvick, git, Angelo Borsotti, Drew Northup,
Michael Haggerty, Philip Oakley, Johannes Sixt, Kacper Kornet,
Felipe Contreras
In-Reply-To: <7vfw21xde5.fsf@alter.siamese.dyndns.org>
On Wed, Jan 16, 2013 at 09:10:10AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I.e., we trigger the "!o" branch after the parse_object in your example.
>
> Heh, I didn't see this message until now (gmane seems to be lagging
> a bit).
I think it is vger lagging, actually.
> I am very tempted to do this.
>
> * Remove unnecessary not_forwardable from "struct ref"; it is only
> used inside set_ref_status_for_push();
>
> * "refs/tags/" is the only hierarchy that cannot be replaced
> without --force;
Agreed.
> * Remove the misguided attempt to force that everything that
> updates an existing ref has to be a commit outside "refs/tags/"
> hierarchy. This code does not know what kind of objects the user
> wants to place in "refs/frotz/" hierarchy it knows nothing about.
I agree with what your patch does, but my thinking is a bit different.
My original suggestion with respect to object types was that the rule
for --force should be "do not ever lose any objects without --force". So
a fast-forward is OK, as the new objects reference the old. A non-fast
forward is not, because objects become unreferenced. Replacing a tag
object is not OK, even if it points to the same commit, as you are
losing the old tag object (replacing an object with a tag that points to
the original object or its descendent is OK in theory, though I doubt it
is common enough to worry about).
I think that is a reasonable rule that could be applied across all parts
of the namespace hierarchy. And it could be applied by the client,
because all you need to know is whether ref->old_sha1 is reachable from
ref->new_sha1.
But it is somewhat orthogonal to the "already exists" idea, and checking
refs/tags/. Those ideas are about enforcing sane rules on the tag
hierarchy. My rule is a safety valve that is meant to extend the idea of
"is fast-forwardable" to non-commit object types. If we do it at all, it
should be part of the fast-forward check (e.g., as part of ref_newer).
The current code conflates the two under the "already exists" condition,
which is just wrong. I think the best thing at this point is to split
the two ideas apart, keep the refs/tags check (and translate it to
"already exists" in the UI, as we do), and table the safety valve. I am
not even sure if it is something that is useful, and it can come later
if we decide it is.
> I feel moderately strongly about the last point. Defining special
> semantics for one hierarchy (e.g. "refs/tags/") and implementing a
> policy for enforcement is one thing, but a random policy that
> depends on object type that applies globally is simply insane. The
> user may want to do "refs/tested/" hierarchy that is meant to hold
> references to commit, with one annotated tag "refs/tested/latest"
> that points at the "latest tested version" with some commentary, and
> maintain the latter by keep pushing to it. If that is the semantics
> the user wanted to ahve in the "refs/tested/" hierarchy, it is not
> reasonable to require --force for such a workflow. The user knows
> better than Git in such a case.
I see what you are saying, but I think the ship has already sailed to
some degree. We already implement the non-fast-forward check everywhere,
and I cannot have a "refs/tested" hierarchy that pushes arbitrary
commits without regard to their history. If I have such a hierarchy, I
have to use "--force" (or more likely, mark the refspec with "+").
In my mind, the object-type checking is just making that fast-forward
check more thorough (i.e., extending it to non-commit objects).
> cache.h | 1 -
> remote.c | 24 +-----------------------
> t/t5516-fetch-push.sh | 21 ---------------------
> 3 files changed, 1 insertion(+), 45 deletions(-)
The patch itself looks fine to me. Whether we agree on the fast-forward
object-type checking or not, it is the correct first step to take in
either case.
-Peff
^ permalink raw reply
* Re: [PATCH] clean.c, ls-files.c: respect encapsulation of exclude_list_groups
From: Junio C Hamano @ 2013-01-16 17:43 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1358342758-30503-1-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> Consumers of the dir.c traversal API should avoid assuming knowledge
> of the internal implementation of exclude_list_groups. Therefore
> when adding items to an exclude list, it should be accessed via the
> pointer returned from add_exclude_list(), rather than by referencing
> a location within dir.exclude_list_groups[EXC_CMDL].
Sounds sensible.
^ permalink raw reply
* Question re. git remote repository
From: Lang, David @ 2013-01-16 17:49 UTC (permalink / raw)
To: 'git@vger.kernel.org'
Hello,
We're just in the process of investigating a versioning tool and are very interesting in git. We have one question we're hoping someone can answer. In regards to the repositories, I think I understand correctly that each developer will have a local repository that they will work from, and that there will also be a remote repository (origin) that will hold the original version of the project.
It appears from the limited reading I've done that the remote repository must be hosted at github.com. Is this the case? Ideally we'd prefer to simply create our remote repository on a drive of one of our local network servers. Is this possible?
Thanks in advance for the help.
David Lang | Application Developer | Tel: 416-340-4800 x.5277
Cardiac IT Dept - Toronto General Hospital
The University Health Network
200 Elizabeth St.
Toronto, ON M5G 2C4
This e-mail may contain confidential and/or privileged information for the sole use of the intended recipient.
Any review or distribution by anyone other than the person for whom it was originally intended is strictly prohibited.
If you have received this e-mail in error, please contact the sender and delete all copies.
Opinions, conclusions or other information contained in this e-mail may not be that of the organization.
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Jeff King @ 2013-01-16 17:50 UTC (permalink / raw)
To: Max Horn
Cc: John Keeping, Antoine Pelisse, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <7FDA1B56-731E-4BA2-8FE5-196B965FFFDB@quendi.de>
On Wed, Jan 16, 2013 at 06:26:35PM +0100, Max Horn wrote:
> > On Wed, Jan 16, 2013 at 06:12:57PM +0100, Antoine Pelisse wrote:
> >> FWIW, I also happen to have the warning:
> >>
> >> advice.c:69:2: warning: expression result unused [-Wunused-value]
> >> error("'%s' is not possible because you have unmerged files.", me);
> >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> ./git-compat-util.h:314:55: note: expanded from:
> >> #define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
> >> ^~
> >>
> >> with clang: Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final)
> >> (based on LLVM 3.0)
> >
> > I have the same output with:
> >
> > clang version 3.2 (tags/RELEASE_32/final)
>
> Sorry for not being more specific in my message. I have this with
>
> Apple clang version 4.1 (tags/Apple/clang-421.11.66) (based on LLVM 3.1svn)
So it seems pretty common, and is just that clang is more concerned
about this than gcc. I think your patch is a reasonable workaround. It
seems a little weird to me that clang defines __GNUC__, but I assume
there are good reasons for it. The commit message should probably be
along the lines of:
Commit a469a10 wraps some error calls in macros to give the compiler a
chance to do more static analysis on their constant -1 return value.
We limit the use of these macros to __GNUC__, since gcc is the primary
beneficiary of the new information, and because we use GNU features
for handling variadic macros.
However, clang also defines __GNUC__, but generates warnings (due to
throwing away the return value from the first half of the macro). We
can squelch the warning by turning off these macros when clang is in
use.
I'm confused, though, why your patch does not have a matching update to
the opterror macro in parse-options.h. It uses exactly the same
technique. Does it not generate a warning?
-Peff
^ permalink raw reply
* [PATCH v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish
From: Martin von Zweigbergk @ 2013-01-16 18:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-18-git-send-email-martinvonz@gmail.com>
---
Sorry, I forgot the documentation updates. I hope this looks ok. Can
you squash this in, Junio? Thanks.
I don't think any documentation update is necessary for the "reset on
unborn branch" patch. Let me know if you think differently.
Documentation/git-reset.txt | 18 +++++++++---------
builtin/reset.c | 4 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 978d8da..a404b47 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,20 +8,20 @@ git-reset - Reset current HEAD to the specified state
SYNOPSIS
--------
[verse]
-'git reset' [-q] [<commit>] [--] <paths>...
-'git reset' (--patch | -p) [<commit>] [--] [<paths>...]
+'git reset' [-q] [<tree-ish>] [--] <paths>...
+'git reset' (--patch | -p) [<tree-sh>] [--] [<paths>...]
'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] [<commit>]
DESCRIPTION
-----------
-In the first and second form, copy entries from <commit> to the index.
+In the first and second form, copy entries from <tree-ish> to the index.
In the third form, set the current branch head (HEAD) to <commit>, optionally
-modifying index and working tree to match. The <commit> defaults to HEAD
-in all forms.
+modifying index and working tree to match. The <tree-ish>/<commit> defaults
+to HEAD in all forms.
-'git reset' [-q] [<commit>] [--] <paths>...::
+'git reset' [-q] [<tree-ish>] [--] <paths>...::
This form resets the index entries for all <paths> to their
- state at <commit>. (It does not affect the working tree, nor
+ state at <tree-ish>. (It does not affect the working tree, nor
the current branch.)
+
This means that `git reset <paths>` is the opposite of `git add
@@ -34,9 +34,9 @@ Alternatively, using linkgit:git-checkout[1] and specifying a commit, you
can copy the contents of a path out of a commit to the index and to the
working tree in one go.
-'git reset' (--patch | -p) [<commit>] [--] [<paths>...]::
+'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]::
Interactively select hunks in the difference between the index
- and <commit> (defaults to HEAD). The chosen hunks are applied
+ and <tree-ish> (defaults to HEAD). The chosen hunks are applied
in reverse to the index.
+
This means that `git reset -p` is the opposite of `git add -p`, i.e.
diff --git a/builtin/reset.c b/builtin/reset.c
index b776867..cb84f1b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -23,8 +23,8 @@
static const char * const git_reset_usage[] = {
N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
- N_("git reset [-q] <commit> [--] <paths>..."),
- N_("git reset --patch [<commit>] [--] [<paths>...]"),
+ N_("git reset [-q] <tree-ish> [--] <paths>..."),
+ N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
NULL
};
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* Re: [PATCH] fix some clang warnings
From: Jeff King @ 2013-01-16 18:00 UTC (permalink / raw)
To: Max Horn
Cc: John Keeping, Antoine Pelisse, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116175057.GB27525@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 09:50:57AM -0800, Jeff King wrote:
> I'm confused, though, why your patch does not have a matching update to
> the opterror macro in parse-options.h. It uses exactly the same
> technique. Does it not generate a warning?
Ah, I think I see why not.
It is not about the macro itself, but rather the callsites that do not
return error, but call it for its printing side effect. It seems that
clang -Wunused-value is OK with unused values from functions being
discarded, but not with constants. So:
int foo();
void bar()
{
foo(); /* ok */
1; /* not ok */
(foo(), 1); /* not ok */
}
The first one is OK (I think it would fall under -Wunused-result under
either compiler). The middle one is an obvious error, and caught by both
compilers. The last one is OK by gcc, but clang complains.
So opterror does not happen to generate any warnings, because we do not
ever use it in a void context. It should probably be marked the same
way, though, as future-proofing.
> The commit message should probably be along the lines of:
> [...]
> However, clang also defines __GNUC__, but generates warnings (due to
> throwing away the return value from the first half of the macro). We
> can squelch the warning by turning off these macros when clang is in
> use.
So a more accurate description would be:
However, clang also defines __GNUC__, but generates warnings with
-Wunused-value when these macros are used in a void context, because
the constant "-1" ends up being useless. Gcc does not complain about
this case (though it is unclear if it is because it is smart enough to
see what we are doing, or too dumb to realize that the -1 is unused).
We can squelch the warning by just disabling these macros when clang
is in use.
-Peff
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Tomas Carnecky @ 2013-01-16 18:03 UTC (permalink / raw)
To: Jeff King, Max Horn
Cc: John Keeping, Antoine Pelisse, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116175057.GB27525@sigill.intra.peff.net>
On Wed, 16 Jan 2013 09:50:57 -0800, Jeff King <peff@peff.net> wrote:
> However, clang also defines __GNUC__, [...]
http://sourceforge.net/p/predef/wiki/Compilers/
Notice that the meaning of the __GNUC__ macro has changed subtly over the
years, from identifying the GNU C/C++ compiler to identifying any compiler
that implements the GNU compiler extensions (...). For example, the Intel
C++ on Linux also defines these macros from version 8.1 (...).
^ permalink raw reply
* Re: Question re. git remote repository
From: Konstantin Khomoutov @ 2013-01-16 18:06 UTC (permalink / raw)
To: Lang, David; +Cc: 'git@vger.kernel.org'
In-Reply-To: <201301161749.r0GHnGV6007806@smtpb02.one-mail.on.ca>
On Wed, 16 Jan 2013 17:49:09 +0000
"Lang, David" <David.Lang@uhn.ca> wrote:
> We're just in the process of investigating a versioning tool and are
> very interesting in git. We have one question we're hoping someone
> can answer. In regards to the repositories, I think I understand
> correctly that each developer will have a local repository that they
> will work from, and that there will also be a remote repository
> (origin) that will hold the original version of the project.
The name "origin" is purely arbitrary: any local repository might have
> It appears from the limited reading I've done that the remote
> repository must be hosted at github.com. Is this the case?
Of course not. github is just a Git hosting provider. There are
plenty of them -- both commercial and not-for-profit (a well-known
service bitbucket.org is one example).
> Ideally we'd prefer to simply create our remote repository on a drive
> of one of our local network servers. Is this possible?
Yes, this is possible, but it's not advised to keep such a "reference"
repository on an exported networked drive for a number of reasons (both
performance and bug-free operation).
Instead, the canonical way to host "reference" repositories is to make
them accessible via SSH or via HTTP[S]. To do this, a server running
some POSIX OS (Linux- or *BSD-based) is the best bet. Both kinds of
access require Git itself installed on the server. Obviously, SSH
access requires an SSH server software (such as OpenSSH) as well and
HTTP[S] access requires a web server (such as Apache). Of course,
everything mentioned is available on any sensible OS you might install
on your server. Read-only access might be provided by a special tool
named "Git daemon" which is a part of Git.
If you have more than a couple of developers you might want to install
certain front-end Git software on the server which provides for
"virtualized" Git users and fine-grained control over who can do what.
Using gitolite [3] for this is the current trend.
Web-browsing for your repositories, if needed, is usually provided by
the tool named gitweb [4].
Everything I've just summarised is well explained in [5] and [6] (as an
addendum).
Another approach is to set up a "turn-key" solution such as GitLab [1]
or gitblit [2].
1. http://gitlabhq.com/
2. http://gitblit.com/
3. https://github.com/sitaramc/gitolite
4. https://git.wiki.kernel.org/index.php/Gitweb
5. http://git-scm.com/book/en/Git-on-the-Server
6. http://git-scm.com/2010/03/04/smart-http.html
^ permalink raw reply
* Re: [PATCH v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish
From: Martin von Zweigbergk @ 2013-01-16 18:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358359235-10213-1-git-send-email-martinvonz@gmail.com>
On Wed, Jan 16, 2013 at 10:00 AM, Martin von Zweigbergk
<martinvonz@gmail.com> wrote:
> ---
>
> Sorry, I forgot the documentation updates. I hope this looks ok. Can
> you squash this in, Junio? Thanks.
I see the series just entered 'next', so I guess it would have to go
on top then. Perhaps with a commit message like as simple as the
following. Let me know if you prefer it to be resent as a proper
patch. Sorry about the noise.
reset: update documentation to require only tree-ish with paths
When resetting with paths, we no longer require a commit argument, but
only a tree-ish. Update the documentation and synopsis accordingly.
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Jeff King @ 2013-01-16 18:09 UTC (permalink / raw)
To: Max Horn
Cc: John Keeping, Antoine Pelisse, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116180041.GC27525@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
> So opterror does not happen to generate any warnings, because we do not
> ever use it in a void context. It should probably be marked the same
> way, though, as future-proofing.
> [...]
> So a more accurate description would be:
Here it is all together:
-- >8 --
From: Max Horn <max@quendi.de>
Subject: [PATCH] fix clang -Wunused-value warnings for error functions
Commit a469a10 wraps some error calls in macros to give the
compiler a chance to do more static analysis on their
constant -1 return value. We limit the use of these macros
to __GNUC__, since gcc is the primary beneficiary of the new
information, and because we use GNU features for handling
variadic macros.
However, clang also defines __GNUC__, but generates warnings
with -Wunused-value when these macros are used in a void
context, because the constant "-1" ends up being useless.
Gcc does not complain about this case (though it is unclear
if it is because it is smart enough to see what we are
doing, or too dumb to realize that the -1 is unused). We
can squelch the warning by just disabling these macros when
clang is in use.
Signed-off-by: Max Horn <max@quendi.de>
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 2 +-
git-compat-util.h | 2 +-
parse-options.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index c257953..5c8440b 100644
--- a/cache.h
+++ b/cache.h
@@ -1148,7 +1148,7 @@ extern int config_error_nonbool(const char *);
extern int git_env_bool(const char *, int);
extern int git_config_system(void);
extern int config_error_nonbool(const char *);
-#ifdef __GNUC__
+#if defined(__GNUC__) && ! defined(__clang__)
#define config_error_nonbool(s) (config_error_nonbool(s), -1)
#endif
extern const char *get_log_output_encoding(void);
diff --git a/git-compat-util.h b/git-compat-util.h
index 2cecf56..2596280 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -297,7 +297,7 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
* behavior. But since we're only trying to help gcc, anyway, it's OK; other
* compilers will fall back to using the function as usual.
*/
-#ifdef __GNUC__
+#if defined(__GNUC__) && ! defined(__clang__)
#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
#endif
diff --git a/parse-options.h b/parse-options.h
index e703853..1c8bd8d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -177,7 +177,7 @@ extern int opterror(const struct option *opt, const char *reason, int flags);
extern int optbug(const struct option *opt, const char *reason);
extern int opterror(const struct option *opt, const char *reason, int flags);
-#ifdef __GNUC__
+#if defined(__GNUC__) && ! defined(clang)
#define opterror(o,r,f) (opterror((o),(r),(f)), -1)
#endif
--
1.8.1.rc1.10.g7d71f7b
^ permalink raw reply related
* Re: [PATCH] fix some clang warnings
From: John Keeping @ 2013-01-16 18:12 UTC (permalink / raw)
To: Jeff King
Cc: Max Horn, John Keeping, Antoine Pelisse, Junio C Hamano, git,
Johannes Sixt
In-Reply-To: <20130116180041.GC27525@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
> It is not about the macro itself, but rather the callsites that do not
> return error, but call it for its printing side effect. It seems that
> clang -Wunused-value is OK with unused values from functions being
> discarded, but not with constants. So:
>
> int foo();
> void bar()
> {
> foo(); /* ok */
> 1; /* not ok */
> (foo(), 1); /* not ok */
> }
>
> The first one is OK (I think it would fall under -Wunused-result under
> either compiler). The middle one is an obvious error, and caught by both
> compilers. The last one is OK by gcc, but clang complains.
I wonder if this would be changed in clang - the change in [1] is
superficially similar.
[1] http://llvm.org/bugs/show_bug.cgi?id=13747
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Matthieu Moy @ 2013-01-16 18:12 UTC (permalink / raw)
To: Jeff King
Cc: Max Horn, John Keeping, Antoine Pelisse, Junio C Hamano, git,
Johannes Sixt
In-Reply-To: <20130116175057.GB27525@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It seems a little weird to me that clang defines __GNUC__, but I
> assume there are good reasons for it.
The reason is essentially that clang targets compatibility with GCC
(implementing the same extensions & cie), in the sense "drop in
replacement that should be able to compile legacy code possibly relying
on __GNUC__ and GCC extensions."
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Jeff King @ 2013-01-16 18:15 UTC (permalink / raw)
To: John Keeping
Cc: Max Horn, Antoine Pelisse, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116181203.GB2476@farnsworth.metanate.com>
On Wed, Jan 16, 2013 at 06:12:03PM +0000, John Keeping wrote:
> On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
> > It is not about the macro itself, but rather the callsites that do not
> > return error, but call it for its printing side effect. It seems that
> > clang -Wunused-value is OK with unused values from functions being
> > discarded, but not with constants. So:
> >
> > int foo();
> > void bar()
> > {
> > foo(); /* ok */
> > 1; /* not ok */
> > (foo(), 1); /* not ok */
> > }
> >
> > The first one is OK (I think it would fall under -Wunused-result under
> > either compiler). The middle one is an obvious error, and caught by both
> > compilers. The last one is OK by gcc, but clang complains.
>
> I wonder if this would be changed in clang - the change in [1] is
> superficially similar.
>
> [1] http://llvm.org/bugs/show_bug.cgi?id=13747
Yeah, I think it is exactly the same issue, and the fix they mention
there would apply to us, too.
Is it worth applying this at all, then? Or should we apply it but limit
it with a clang version macro (they mention r163034, but I do not know
if it is in a released version yet, nor what macros are available to
inspect the version)?
-Peff
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Antoine Pelisse @ 2013-01-16 18:21 UTC (permalink / raw)
To: Jeff King; +Cc: John Keeping, Max Horn, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116181558.GA4426@sigill.intra.peff.net>
> Is it worth applying this at all, then? Or should we apply it but limit
> it with a clang version macro (they mention r163034, but I do not know
> if it is in a released version yet, nor what macros are available to
> inspect the version)?
Please also note that building with clang is not warning-free (though
I think it would be nice)
^ permalink raw reply
* Re: Question re. git remote repository
From: Jeff King @ 2013-01-16 18:21 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: Lang, David, 'git@vger.kernel.org'
In-Reply-To: <20130116220615.48c159546bccfa5b9cd9028e@domain007.com>
On Wed, Jan 16, 2013 at 10:06:15PM +0400, Konstantin Khomoutov wrote:
> > In regards to the repositories, I think I understand correctly that
> > each developer will have a local repository that they will work
> > from, and that there will also be a remote repository (origin) that
> > will hold the original version of the project.
Note that this is just one common topology for setting up repos (and it
is probably the simplest). Others are described here:
http://git-scm.com/book/en/Distributed-Git-Distributed-Workflows
> > Ideally we'd prefer to simply create our remote repository on a drive
> > of one of our local network servers. Is this possible?
>
> Yes, this is possible, but it's not advised to keep such a "reference"
> repository on an exported networked drive for a number of reasons (both
> performance and bug-free operation).
I agree that performance is not ideal (although if you are on a fast
LAN, it probably would not matter much), but I do not recall any
specific bugs in that area. Can you elaborate?
-Peff
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: John Keeping @ 2013-01-16 18:22 UTC (permalink / raw)
To: Jeff King
Cc: John Keeping, Max Horn, Antoine Pelisse, Junio C Hamano, git,
Johannes Sixt
In-Reply-To: <20130116181558.GA4426@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 10:15:58AM -0800, Jeff King wrote:
> On Wed, Jan 16, 2013 at 06:12:03PM +0000, John Keeping wrote:
>
> > On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
> > > It is not about the macro itself, but rather the callsites that do not
> > > return error, but call it for its printing side effect. It seems that
> > > clang -Wunused-value is OK with unused values from functions being
> > > discarded, but not with constants. So:
> > >
> > > int foo();
> > > void bar()
> > > {
> > > foo(); /* ok */
> > > 1; /* not ok */
> > > (foo(), 1); /* not ok */
> > > }
> > >
> > > The first one is OK (I think it would fall under -Wunused-result under
> > > either compiler). The middle one is an obvious error, and caught by both
> > > compilers. The last one is OK by gcc, but clang complains.
> >
> > I wonder if this would be changed in clang - the change in [1] is
> > superficially similar.
> >
> > [1] http://llvm.org/bugs/show_bug.cgi?id=13747
>
> Yeah, I think it is exactly the same issue, and the fix they mention
> there would apply to us, too.
>
> Is it worth applying this at all, then? Or should we apply it but limit
> it with a clang version macro (they mention r163034, but I do not know
> if it is in a released version yet, nor what macros are available to
> inspect the version)?
That maps to revision 06b3a06007 in their git repository [1], which is
contained in remotes/origin/release_32 so I think that change should be
in release 3.2, where I still see the warning (although that's not using
a clang built from that source), so I don't think that the fix for that
bug removes the warning in this case.
[1] http://llvm.org/git/clang.git
^ permalink raw reply
* Re: git fetch without --recurse-submodules option
From: Jens Lehmann @ 2013-01-16 18:24 UTC (permalink / raw)
To: 乙酸鋰; +Cc: git
In-Reply-To: <CAHtLG6RE-xGYsp-Apcu3hk8OVck+HLYZdBtWvcweaBNetwtKNA@mail.gmail.com>
Am 16.01.2013 06:45, schrieb 乙酸鋰:
> With git pull or git fetch without specifying --recurse-submodules,
> what is the default action?
on-demand fetch (unless something else is configured).
> It seems git fetches submodules wtihout specifying --recurse-submodules.
> If this is not clear, please update documentation.
You are right, the documentation for pull and fetch does not state
explicitly that "on-demand" is the default here when the option is
not used.
> In git pull document --recurse-submodules option, it tells users to
> see git-config(1) and gitmodules(5), but does not tell users to refer
> to git fetch --recurse-submodules for the meaning of the switches.
> In git fetch document --recurse-submodules option, it does not tell
> users to see git-config(1) or gitmodules(5).
Thanks for pointing that out. Unless anyone else wants to improve the
documentation I'll look into that in my next git time slot.
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Jeff King @ 2013-01-16 18:24 UTC (permalink / raw)
To: John Keeping
Cc: Max Horn, Antoine Pelisse, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116182240.GC2476@farnsworth.metanate.com>
On Wed, Jan 16, 2013 at 06:22:40PM +0000, John Keeping wrote:
> > > [1] http://llvm.org/bugs/show_bug.cgi?id=13747
> >
> > Yeah, I think it is exactly the same issue, and the fix they mention
> > there would apply to us, too.
> >
> > Is it worth applying this at all, then? Or should we apply it but limit
> > it with a clang version macro (they mention r163034, but I do not know
> > if it is in a released version yet, nor what macros are available to
> > inspect the version)?
>
> That maps to revision 06b3a06007 in their git repository [1], which is
> contained in remotes/origin/release_32 so I think that change should be
> in release 3.2, where I still see the warning (although that's not using
> a clang built from that source), so I don't think that the fix for that
> bug removes the warning in this case.
>
> [1] http://llvm.org/git/clang.git
Thanks for checking. I'd rather squelch the warning completely (as in my
re-post of Max's patch from a few minutes ago), and we can loosen it
(possibly with a version check) later when a fix is widely disseminated.
I know that compiling git with clang is not warning-free yet, but it is
close, and I do not mind if somebody puts some effort into making it so.
This gets us one step closer.
-Peff
^ permalink raw reply
* Re: [PATCH v3] test-lib.sh: unfilter GIT_PERF_*
From: Jonathan Nieder @ 2013-01-16 18:54 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Thomas Rast
In-Reply-To: <1358331833-8289-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy wrote:
> These variables are user parameters to control how to run the perf
> tests. Allow users to do so.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
FWIW, of the three versions proposed, this one makes the most sense to
me. Looks good.
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: John Keeping @ 2013-01-16 19:01 UTC (permalink / raw)
To: Jeff King
Cc: John Keeping, Max Horn, Antoine Pelisse, Junio C Hamano, git,
Johannes Sixt
In-Reply-To: <20130116182449.GA4881@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 10:24:49AM -0800, Jeff King wrote:
> On Wed, Jan 16, 2013 at 06:22:40PM +0000, John Keeping wrote:
>
> > > > [1] http://llvm.org/bugs/show_bug.cgi?id=13747
> > >
> > > Yeah, I think it is exactly the same issue, and the fix they mention
> > > there would apply to us, too.
> > >
> > > Is it worth applying this at all, then? Or should we apply it but limit
> > > it with a clang version macro (they mention r163034, but I do not know
> > > if it is in a released version yet, nor what macros are available to
> > > inspect the version)?
> >
> > That maps to revision 06b3a06007 in their git repository [1], which is
> > contained in remotes/origin/release_32 so I think that change should be
> > in release 3.2, where I still see the warning (although that's not using
> > a clang built from that source), so I don't think that the fix for that
> > bug removes the warning in this case.
> >
> > [1] http://llvm.org/git/clang.git
>
> Thanks for checking. I'd rather squelch the warning completely (as in my
> re-post of Max's patch from a few minutes ago), and we can loosen it
> (possibly with a version check) later when a fix is widely disseminated.
I checked again with a trunk build of clang and the warning's still
there, so I've created a clang bug [1] to see if they will change the
behaviour.
I agree that we should squelch the warning for now, it can be changed
into a version check if it's accepted as a bug and once we know what
version it's fixed in.
[1] http://llvm.org/bugs/show_bug.cgi?id=14968
^ permalink raw reply
* Re: [PATCH] attr: fix off-by-one directory component length calculation
From: Junio C Hamano @ 2013-01-16 19:07 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jean-Noël AVILA, git
In-Reply-To: <20130116060238.GA29523@duynguyen-vnpc.dek-tpc.internal>
Duy Nguyen <pclouds@gmail.com> writes:
> OK I get your point now. Something like this?
>
> -- 8< --
> Subject: [PATCH] attr: avoid calling find_basename() twice per path
>
> find_basename() is only used inside collect_all_attrs(), called once
> in prepare_attr_stack, then again after prepare_attr_stack()
> returns. Both calls return exact same value. Reorder the code to do
> the same task once. Also avoid strlen() because we knows the length
> after finding basename.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Yeah, I think this is a nice code reduction, readability improvement
and micro optimization rolled into one.
> attr.c | 45 ++++++++++++++++++---------------------------
> 1 file changed, 18 insertions(+), 27 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index cfc6748..880f862 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -564,32 +564,12 @@ static void bootstrap_attr_stack(void)
> attr_stack = elem;
> }
>
> -static const char *find_basename(const char *path)
> -{
> - const char *cp, *last_slash = NULL;
> -
> - for (cp = path; *cp; cp++) {
> - if (*cp == '/' && cp[1])
> - last_slash = cp;
> - }
> - return last_slash ? last_slash + 1 : path;
> -}
> -
> -static void prepare_attr_stack(const char *path)
> +static void prepare_attr_stack(const char *path, int dirlen)
> {
> struct attr_stack *elem, *info;
> - int dirlen, len;
> + int len;
> const char *cp;
>
> - dirlen = find_basename(path) - path;
> -
> - /*
> - * find_basename() includes the trailing slash, but we do
> - * _not_ want it.
> - */
> - if (dirlen)
> - dirlen--;
> -
> /*
> * At the bottom of the attribute stack is the built-in
> * set of attribute definitions, followed by the contents
> @@ -769,15 +749,26 @@ static int macroexpand_one(int attr_nr, int rem)
> static void collect_all_attrs(const char *path)
> {
> struct attr_stack *stk;
> - int i, pathlen, rem;
> - const char *basename;
> + int i, pathlen, rem, dirlen;
> + const char *basename, *cp, *last_slash = NULL;
> +
> + for (cp = path; *cp; cp++) {
> + if (*cp == '/' && cp[1])
> + last_slash = cp;
> + }
> + pathlen = cp - path;
> + if (last_slash) {
> + basename = last_slash + 1;
> + dirlen = last_slash - path;
> + } else {
> + basename = path;
> + dirlen = 0;
> + }
>
> - prepare_attr_stack(path);
> + prepare_attr_stack(path, dirlen);
> for (i = 0; i < attr_nr; i++)
> check_all_attr[i].value = ATTR__UNKNOWN;
>
> - basename = find_basename(path);
> - pathlen = strlen(path);
> rem = attr_nr;
> for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
> rem = fill(path, pathlen, basename, stk, rem);
^ permalink raw reply
* [PATCH v2] Allow custom "comment char"
From: Ralf Thielow @ 2013-01-16 19:18 UTC (permalink / raw)
To: gitster; +Cc: jrnieder, git, Ralf Thielow
In-Reply-To: <1358275827-5244-1-git-send-email-ralf.thielow@gmail.com>
From: Junio C Hamano <gitster@pobox.com>
Some users do want to write a line that begin with a pound sign, #,
in their commit log message. Many tracking system recognise
a token of #<bugid> form, for example.
The support we offer these use cases is not very friendly to the end
users. They have a choice between
- Don't do it. Avoid such a line by rewrapping or indenting; and
- Use --cleanup=whitespace but remove all the hint lines we add.
Give them a way to set a custom comment char, e.g.
$ git -c core.commentchar="%" commit
so that they do not have to do either of the two workarounds.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
Junio, thanks for the code in your reply to the
first version. It works very well and looks nice.
I was also unhappy about this "\n%c\n" thing and
pretty unsure with the code in "git-submodule.sh".
But with this, it looks good to me. Thanks.
Changes in v2:
- extend "git stripspace" with an option to make
it's input being converted to commented lines
- teach git-submodule.sh using this
- rename strbuf_commented_addstr to strbuf_add_commented_lines
and improve it's design
Documentation/config.txt | 6 ++++
Documentation/git-stripspace.txt | 8 ++++-
Documentation/technical/api-strbuf.txt | 10 ++++++
builtin/branch.c | 10 +++---
builtin/commit.c | 12 +++----
builtin/fmt-merge-msg.c | 2 +-
builtin/merge.c | 5 ++-
builtin/notes.c | 34 +++++++++-----------
builtin/stripspace.c | 48 +++++++++++++++++++++++-----
builtin/tag.c | 34 ++++++++++----------
cache.h | 6 ++++
config.c | 8 +++++
environment.c | 6 ++++
git-submodule.sh | 8 ++---
strbuf.c | 58 ++++++++++++++++++++++++++++------
strbuf.h | 4 +++
t/t0030-stripspace.sh | 35 ++++++++++++++++++++
t/t7502-commit.sh | 7 ++++
t/t7508-status.sh | 50 +++++++++++++++++++++++++++++
wt-status.c | 10 +++---
20 files changed, 283 insertions(+), 78 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5809e0..e99b9f2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -528,6 +528,12 @@ core.editor::
variable when it is set, and the environment variable
`GIT_EDITOR` is not set. See linkgit:git-var[1].
+core.commentchar::
+ Commands such as `commit` and `tag` that lets you edit
+ messages consider a line that begins with this character
+ commented, and removes them after the editor returns
+ (default '#').
+
sequence.editor::
Text editor used by `git rebase -i` for editing the rebase insn file.
The value is meant to be interpreted by the shell when it is used.
diff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt
index a80d946..e6fdfcb 100644
--- a/Documentation/git-stripspace.txt
+++ b/Documentation/git-stripspace.txt
@@ -35,7 +35,13 @@ OPTIONS
-------
-s::
--strip-comments::
- Skip and remove all lines starting with '#'.
+ Skip and remove all lines starting with comment character (default '#').
+
+-c::
+--comment-lines::
+ Prepend comment character and blank to each line. Lines will automatically
+ be terminated with a newline. On empty lines, only the comment character
+ will be prepended.
EXAMPLES
--------
diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index 84686b5..2c59cb2 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -156,6 +156,11 @@ then they will free() it.
Remove the bytes between `pos..pos+len` and replace it with the given
data.
+`strbuf_add_commented_lines`::
+
+ Add a NUL-terminated string to the buffer. Each line will be prepended
+ by a comment character and a blank.
+
`strbuf_add`::
Add data of given length to the buffer.
@@ -229,6 +234,11 @@ which can be used by the programmer of the callback as she sees fit.
Add a formatted string to the buffer.
+`strbuf_commented_addf`::
+
+ Add a formatted string prepended by a comment character and a
+ blank to the buffer.
+
`strbuf_fread`::
Read a given size of data from a FILE* pointer to the buffer.
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..3548271 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -706,11 +706,11 @@ static int edit_branch_description(const char *branch_name)
read_branch_desc(&buf, branch_name);
if (!buf.len || buf.buf[buf.len-1] != '\n')
strbuf_addch(&buf, '\n');
- strbuf_addf(&buf,
- "# Please edit the description for the branch\n"
- "# %s\n"
- "# Lines starting with '#' will be stripped.\n",
- branch_name);
+ strbuf_commented_addf(&buf,
+ "Please edit the description for the branch\n"
+ " %s\n"
+ "Lines starting with '%c' will be stripped.\n",
+ branch_name, comment_line_char);
fp = fopen(git_path(edit_description), "w");
if ((fwrite(buf.buf, 1, buf.len, fp) < buf.len) || fclose(fp)) {
strbuf_release(&buf);
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..f95f64a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -733,15 +733,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (cleanup_mode == CLEANUP_ALL)
status_printf(s, GIT_COLOR_NORMAL,
_("Please enter the commit message for your changes."
- " Lines starting\nwith '#' will be ignored, and an empty"
- " message aborts the commit.\n"));
+ " Lines starting\nwith '%c' will be ignored, and an empty"
+ " message aborts the commit.\n"), comment_line_char);
else /* CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
_("Please enter the commit message for your changes."
- " Lines starting\n"
- "with '#' will be kept; you may remove them"
- " yourself if you want to.\n"
- "An empty message aborts the commit.\n"));
+ " Lines starting\n"
+ "with '%c' will be kept; you may remove them"
+ " yourself if you want to.\n"
+ "An empty message aborts the commit.\n"), comment_line_char);
if (only_include_assumed)
status_printf_ln(s, GIT_COLOR_NORMAL,
"%s", only_include_assumed);
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index d9af43c..b49612f 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -470,7 +470,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,
strbuf_complete_line(tagbuf);
if (sig->len) {
strbuf_addch(tagbuf, '\n');
- strbuf_add_lines(tagbuf, "# ", sig->buf, sig->len);
+ strbuf_add_commented_lines(tagbuf, sig->buf, sig->len);
}
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 9307e9c..7c8922c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -788,17 +788,16 @@ static const char merge_editor_comment[] =
N_("Please enter a commit message to explain why this merge is necessary,\n"
"especially if it merges an updated upstream into a topic branch.\n"
"\n"
- "Lines starting with '#' will be ignored, and an empty message aborts\n"
+ "Lines starting with '%c' will be ignored, and an empty message aborts\n"
"the commit.\n");
static void prepare_to_commit(struct commit_list *remoteheads)
{
struct strbuf msg = STRBUF_INIT;
- const char *comment = _(merge_editor_comment);
strbuf_addbuf(&msg, &merge_msg);
strbuf_addch(&msg, '\n');
if (0 < option_edit)
- strbuf_add_lines(&msg, "# ", comment, strlen(comment));
+ strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
write_merge_msg(&msg);
if (run_hook(get_index_file(), "prepare-commit-msg",
git_path("MERGE_MSG"), "merge", NULL, NULL))
diff --git a/builtin/notes.c b/builtin/notes.c
index 453457a..57748a6 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -92,10 +92,7 @@ static const char * const git_notes_get_ref_usage[] = {
};
static const char note_template[] =
- "\n"
- "#\n"
- "# Write/edit the notes for the following object:\n"
- "#\n";
+ "\nWrite/edit the notes for the following object:\n";
struct msg_arg {
int given;
@@ -129,7 +126,7 @@ static void write_commented_object(int fd, const unsigned char *object)
{"show", "--stat", "--no-notes", sha1_to_hex(object), NULL};
struct child_process show;
struct strbuf buf = STRBUF_INIT;
- FILE *show_out;
+ struct strbuf cbuf = STRBUF_INIT;
/* Invoke "git show --stat --no-notes $object" */
memset(&show, 0, sizeof(show));
@@ -142,21 +139,14 @@ static void write_commented_object(int fd, const unsigned char *object)
die(_("unable to start 'show' for object '%s'"),
sha1_to_hex(object));
- /* Open the output as FILE* so strbuf_getline() can be used. */
- show_out = xfdopen(show.out, "r");
- if (show_out == NULL)
- die_errno(_("can't fdopen 'show' output fd"));
+ if (strbuf_read(&buf, show.out, 0) < 0)
+ die_errno(_("could not read 'show' output"));
+ strbuf_add_commented_lines(&cbuf, buf.buf, buf.len);
+ write_or_die(fd, cbuf.buf, cbuf.len);
- /* Prepend "# " to each output line and write result to 'fd' */
- while (strbuf_getline(&buf, show_out, '\n') != EOF) {
- write_or_die(fd, "# ", 2);
- write_or_die(fd, buf.buf, buf.len);
- write_or_die(fd, "\n", 1);
- }
+ strbuf_release(&cbuf);
strbuf_release(&buf);
- if (fclose(show_out))
- die_errno(_("failed to close pipe to 'show' for object '%s'"),
- sha1_to_hex(object));
+
if (finish_command(&show))
die(_("failed to finish 'show' for object '%s'"),
sha1_to_hex(object));
@@ -170,6 +160,7 @@ static void create_note(const unsigned char *object, struct msg_arg *msg,
if (msg->use_editor || !msg->given) {
int fd;
+ struct strbuf buf = STRBUF_INIT;
/* write the template message before editing: */
path = git_pathdup("NOTES_EDITMSG");
@@ -181,11 +172,16 @@ static void create_note(const unsigned char *object, struct msg_arg *msg,
write_or_die(fd, msg->buf.buf, msg->buf.len);
else if (prev && !append_only)
write_note_data(fd, prev);
- write_or_die(fd, note_template, strlen(note_template));
+
+ strbuf_addch(&buf, '\n');
+ strbuf_add_commented_lines(&buf, note_template, strlen(note_template));
+ strbuf_addch(&buf, '\n');
+ write_or_die(fd, buf.buf, buf.len);
write_commented_object(fd, object);
close(fd);
+ strbuf_release(&buf);
strbuf_reset(&(msg->buf));
if (launch_editor(path, &(msg->buf), NULL)) {
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index f16986c..234248b 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -30,7 +30,8 @@ static size_t cleanup(char *line, size_t len)
*
* If last line does not have a newline at the end, one is added.
*
- * Enable skip_comments to skip every line starting with "#".
+ * Enable skip_comments to skip every line starting with comment
+ * character.
*/
void stripspace(struct strbuf *sb, int skip_comments)
{
@@ -45,7 +46,7 @@ void stripspace(struct strbuf *sb, int skip_comments)
eol = memchr(sb->buf + i, '\n', sb->len - i);
len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
- if (skip_comments && len && sb->buf[i] == '#') {
+ if (skip_comments && len && sb->buf[i] == comment_line_char) {
newlen = 0;
continue;
}
@@ -66,21 +67,52 @@ void stripspace(struct strbuf *sb, int skip_comments)
strbuf_setlen(sb, j);
}
+static void comment_lines(struct strbuf *buf)
+{
+ char *msg;
+ size_t len;
+
+ msg = strbuf_detach(buf, &len);
+ strbuf_add_commented_lines(buf, msg, len);
+}
+
+static const char *usage_msg = "\n"
+" git stripspace [-s | --strip-comments] < input\n"
+" git stripspace [-c | --comment-lines] < input";
+
int cmd_stripspace(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
int strip_comments = 0;
+ enum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = STRIP_SPACE;
+
+ if (argc == 2) {
+ if (!strcmp(argv[1], "-s") ||
+ !strcmp(argv[1], "--strip-comments")) {
+ strip_comments = 1;
+ } else if (!strcmp(argv[1], "-c") ||
+ !strcmp(argv[1], "--comment-lines")) {
+ mode = COMMENT_LINES;
+ } else {
+ mode = INVAL;
+ }
+ } else if (argc > 1) {
+ mode = INVAL;
+ }
+
+ if (mode == INVAL)
+ usage(usage_msg);
- if (argc == 2 && (!strcmp(argv[1], "-s") ||
- !strcmp(argv[1], "--strip-comments")))
- strip_comments = 1;
- else if (argc > 1)
- usage("git stripspace [-s | --strip-comments] < input");
+ if (strip_comments || mode == COMMENT_LINES)
+ git_config(git_default_config, NULL);
if (strbuf_read(&buf, 0, 1024) < 0)
die_errno("could not read the input");
- stripspace(&buf, strip_comments);
+ if (mode == STRIP_SPACE)
+ stripspace(&buf, strip_comments);
+ else
+ comment_lines(&buf);
write_or_die(1, buf.buf, buf.len);
strbuf_release(&buf);
diff --git a/builtin/tag.c b/builtin/tag.c
index 9c3e067..f826688 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -246,19 +246,13 @@ static int do_sign(struct strbuf *buffer)
}
static const char tag_template[] =
- N_("\n"
- "#\n"
- "# Write a tag message\n"
- "# Lines starting with '#' will be ignored.\n"
- "#\n");
+ N_("\nWrite a tag message\n"
+ "Lines starting with '%c' will be ignored.\n");
static const char tag_template_nocleanup[] =
- N_("\n"
- "#\n"
- "# Write a tag message\n"
- "# Lines starting with '#' will be kept; you may remove them"
- " yourself if you want to.\n"
- "#\n");
+ N_("\nWrite a tag message\n"
+ "Lines starting with '%c' will be kept; you may remove them"
+ " yourself if you want to.\n");
static int git_tag_config(const char *var, const char *value, void *cb)
{
@@ -346,14 +340,18 @@ static void create_tag(const unsigned char *object, const char *tag,
if (fd < 0)
die_errno(_("could not create file '%s'"), path);
- if (!is_null_sha1(prev))
+ if (!is_null_sha1(prev)) {
write_tag_body(fd, prev);
- else if (opt->cleanup_mode == CLEANUP_ALL)
- write_or_die(fd, _(tag_template),
- strlen(_(tag_template)));
- else
- write_or_die(fd, _(tag_template_nocleanup),
- strlen(_(tag_template_nocleanup)));
+ } else {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addch(&buf, '\n');
+ if (opt->cleanup_mode == CLEANUP_ALL)
+ strbuf_commented_addf(&buf, _(tag_template), comment_line_char);
+ else
+ strbuf_commented_addf(&buf, _(tag_template_nocleanup), comment_line_char);
+ write_or_die(fd, buf.buf, buf.len);
+ strbuf_release(&buf);
+ }
close(fd);
if (launch_editor(path, buf, NULL)) {
diff --git a/cache.h b/cache.h
index c257953..0b435a4 100644
--- a/cache.h
+++ b/cache.h
@@ -562,6 +562,12 @@ extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
+/*
+ * The character that begins a commented line in user-editable file
+ * that is subject to stripspace.
+ */
+extern char comment_line_char;
+
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
diff --git a/config.c b/config.c
index 7b444b6..d873c59 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,14 @@ static int git_default_core_config(const char *var, const char *value)
if (!strcmp(var, "core.editor"))
return git_config_string(&editor_program, var, value);
+ if (!strcmp(var, "core.commentchar")) {
+ const char *comment;
+ int ret = git_config_string(&comment, var, value);
+ if (!ret)
+ comment_line_char = comment[0];
+ return ret;
+ }
+
if (!strcmp(var, "core.askpass"))
return git_config_string(&askpass_program, var, value);
diff --git a/environment.c b/environment.c
index 85edd7f..a40c38b 100644
--- a/environment.c
+++ b/environment.c
@@ -62,6 +62,12 @@ int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
struct startup_info *startup_info;
unsigned long pack_size_limit_cfg;
+/*
+ * The character that begins a commented line in user-editable file
+ * that is subject to stripspace.
+ */
+char comment_line_char = '#';
+
/* Parallel index stat data preload? */
int core_preload_index = 0;
diff --git a/git-submodule.sh b/git-submodule.sh
index 22ec5b6..004c034 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -976,12 +976,12 @@ cmd_summary() {
done |
if test -n "$for_status"; then
if [ -n "$files" ]; then
- gettextln "# Submodules changed but not updated:"
+ gettextln "Submodules changed but not updated:" | git stripspace -c
else
- gettextln "# Submodule changes to be committed:"
+ gettextln "Submodule changes to be committed:" | git stripspace -c
fi
- echo "#"
- sed -e 's|^|# |' -e 's|^# $|#|'
+ printf "\n" | git stripspace -c
+ git stripspace -c
else
cat
fi
diff --git a/strbuf.c b/strbuf.c
index 9a373be..48e9abb 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -204,6 +204,54 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
va_end(ap);
}
+static void add_lines(struct strbuf *out,
+ const char *prefix1,
+ const char *prefix2,
+ const char *buf, size_t size)
+{
+ while (size) {
+ const char *prefix;
+ const char *next = memchr(buf, '\n', size);
+ next = next ? (next + 1) : (buf + size);
+
+ prefix = (prefix2 && buf[0] == '\n') ? prefix2 : prefix1;
+ strbuf_addstr(out, prefix);
+ strbuf_add(out, buf, next - buf);
+ size -= next - buf;
+ buf = next;
+ }
+ strbuf_complete_line(out);
+}
+
+void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size)
+{
+ static char prefix1[3];
+ static char prefix2[2];
+
+ if (prefix1[0] != comment_line_char) {
+ sprintf(prefix1, "%c ", comment_line_char);
+ sprintf(prefix2, "%c", comment_line_char);
+ }
+ add_lines(out, prefix1, prefix2, buf, size);
+}
+
+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...)
+{
+ va_list params;
+ struct strbuf buf = STRBUF_INIT;
+ int incomplete_line = sb->len && sb->buf[sb->len - 1] != '\n';
+
+ va_start(params, fmt);
+ strbuf_vaddf(&buf, fmt, params);
+ va_end(params);
+
+ strbuf_add_commented_lines(sb, buf.buf, buf.len);
+ if (incomplete_line)
+ sb->buf[--sb->len] = '\0';
+
+ strbuf_release(&buf);
+}
+
void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
{
int len;
@@ -414,15 +462,7 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
void strbuf_add_lines(struct strbuf *out, const char *prefix,
const char *buf, size_t size)
{
- while (size) {
- const char *next = memchr(buf, '\n', size);
- next = next ? (next + 1) : (buf + size);
- strbuf_addstr(out, prefix);
- strbuf_add(out, buf, next - buf);
- size -= next - buf;
- buf = next;
- }
- strbuf_complete_line(out);
+ add_lines(out, prefix, NULL, buf, size);
}
void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)
diff --git a/strbuf.h b/strbuf.h
index ecae4e2..958822c 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -110,6 +110,8 @@ extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);
extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,
const void *, size_t);
+extern void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size);
+
extern void strbuf_add(struct strbuf *, const void *, size_t);
static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
strbuf_add(sb, s, strlen(s));
@@ -131,6 +133,8 @@ extern void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *
__attribute__((format (printf,2,3)))
extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
+__attribute__((format (printf, 2, 3)))
+extern void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
__attribute__((format (printf,2,0)))
extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index ccb0a3c..a8e84d8 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -397,4 +397,39 @@ test_expect_success 'strip comments, too' '
test -z "$(echo "# comment" | git stripspace -s)"
'
+test_expect_success 'strip comments with changed comment char' '
+ test ! -z "$(echo "; comment" | git -c core.commentchar=";" stripspace)" &&
+ test -z "$(echo "; comment" | git -c core.commentchar=";" stripspace -s)"
+'
+
+test_expect_success '-c with single line' '
+ printf "# foo\n" >expect &&
+ printf "foo" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '-c with single line followed by empty line' '
+ printf "# foo\n#\n" >expect &&
+ printf "foo\n\n" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '-c with newline only' '
+ printf "#\n" >expect &&
+ printf "\n" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--comment-lines with single line' '
+ printf "# foo\n" >expect &&
+ printf "foo" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '-c with changed comment char' '
+ printf "; foo\n" >expect &&
+ printf "foo" | git -c core.commentchar=";" stripspace -c >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 1a5cb69..6a2c67c 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -447,4 +447,11 @@ use_template="-t template"
try_commit_status_combo
+test_expect_success 'commit --status with custom comment character' '
+ test_when_finished "git config --unset core.commentchar" &&
+ git config core.commentchar ";" &&
+ try_commit --status &&
+ test_i18ngrep "^; Changes to be committed:" .git/COMMIT_EDITMSG
+'
+
test_done
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e313ef1..a79c032 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1254,6 +1254,56 @@ test_expect_success ".git/config ignore=dirty doesn't suppress submodule summary
'
cat > expect << EOF
+; On branch master
+; Changes to be committed:
+; (use "git reset HEAD <file>..." to unstage)
+;
+; modified: sm
+;
+; Changes not staged for commit:
+; (use "git add <file>..." to update what will be committed)
+; (use "git checkout -- <file>..." to discard changes in working directory)
+;
+; modified: dir1/modified
+; modified: sm (new commits)
+;
+; Submodule changes to be committed:
+;
+; * sm $head...$new_head (1):
+; > Add bar
+;
+; Submodules changed but not updated:
+;
+; * sm $new_head...$head2 (1):
+; > 2nd commit
+;
+; Untracked files:
+; (use "git add <file>..." to include in what will be committed)
+;
+; .gitmodules
+; dir1/untracked
+; dir2/modified
+; dir2/untracked
+; expect
+; output
+; untracked
+EOF
+
+test_expect_success "status (core.commentchar with submodule summary)" '
+ test_when_finished "git config --unset core.commentchar" &&
+ git config core.commentchar ";" &&
+ git status >output &&
+ test_i18ncmp expect output
+'
+
+test_expect_success "status (core.commentchar with two chars with submodule summary)" '
+ test_when_finished "git config --unset core.commentchar" &&
+ git config core.commentchar ";;" &&
+ git status >output &&
+ test_i18ncmp expect output
+'
+
+cat > expect << EOF
# On branch master
# Changes not staged for commit:
# (use "git add <file>..." to update what will be committed)
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..f6f197e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -45,7 +45,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
strbuf_vaddf(&sb, fmt, ap);
if (!sb.len) {
- strbuf_addch(&sb, '#');
+ strbuf_addch(&sb, comment_line_char);
if (!trail)
strbuf_addch(&sb, ' ');
color_print_strbuf(s->fp, color, &sb);
@@ -59,7 +59,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
strbuf_reset(&linebuf);
if (at_bol) {
- strbuf_addch(&linebuf, '#');
+ strbuf_addch(&linebuf, comment_line_char);
if (*line != '\n' && *line != '\t')
strbuf_addch(&linebuf, ' ');
}
@@ -760,8 +760,10 @@ static void wt_status_print_tracking(struct wt_status *s)
for (cp = sb.buf; (ep = strchr(cp, '\n')) != NULL; cp = ep + 1)
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s),
- "# %.*s", (int)(ep - cp), cp);
- color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
+ "%c %.*s", comment_line_char,
+ (int)(ep - cp), cp);
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c",
+ comment_line_char);
}
static int has_unmerged(struct wt_status *s)
--
1.8.1.291.g056e2eb
^ permalink raw reply related
* Re: [PATCH] git-remote: distinguish between default and configured URLs
From: Junio C Hamano @ 2013-01-16 19:19 UTC (permalink / raw)
To: Michael J Gruber; +Cc: John Keeping, git, Jardel Weyrich, Sascha Cunz
In-Reply-To: <50F6A0F0.70800@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> In short, the separate listing is correct, but in this case there's no
> improvement in readability.
Yes, I think the "insteadOf" rewrite is a related but a separate
issue.
Is "remote -v" meant for diagnosing remote.origin.{url,pushurl} that
are misconfigured?
If not, the output just should just say the final outcome, i.e. what
destinations we will fetch from and push to, without cluttering the
output.
If on the other hand it is to help users debug their configuration,
the output also needs to explain exactly what made us decide those
destinations to use (e.g. to discover there was a leftover insteadof
in $HOME/.gitconfig the user forgot about).
^ permalink raw reply
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Andreas Schwab @ 2013-01-16 19:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael J Gruber, Jardel Weyrich, Sascha Cunz,
git@vger.kernel.org
In-Reply-To: <7v622xyvnd.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I actually think my earlier "it shouldn't be the same (push)" is not
> needed and probably is actively wrong. Just like you can tell
> between
>
> (only one .url) (both .url and .pushurl)
>
> origin there (fetch/push) origin there (fetch)
> origin there (push)
What should happen when you have a .pushinsteadof configured that
modifies .url for pushing?
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox