* Re: Bug in "git log --graph -p -m" (version 1.7.7.6)
From: John Keeping @ 2013-02-06 15:14 UTC (permalink / raw)
To: Dale R. Worley; +Cc: Matthieu Moy, gitster, git
In-Reply-To: <201302061503.r16F30UA016375@freeze.ariadne.com>
On Wed, Feb 06, 2013 at 10:03:00AM -0500, Dale R. Worley wrote:
> > From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> >
> > In any case, I can't reproduce with 1.8.1.2.526.gf51a757: I don't get
> > undless output. On the other hand, I get a slightly misformatted output:
> >
> > * commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 2c1e6a36f4b712e914fac994463da7d0fdb2bc6d)
> > |\ Merge: 2c1e6a3 33e70e7
> > | | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
> > | | Date: Tue Feb 5 22:05:33 2013 +0100
> > | |
> > | | Commit S
> > | |
> > | | diff --git a/file b/file
> > | | index 6bb4d3e..afd2e75 100644
> > | | --- a/file
> > | | +++ b/file
> > | | @@ -1,4 +1,5 @@
> > | | 1
> > | | 1a
> > | | 2
> > | | +2a
> > | | 3
> > | |
> > commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 33e70e70c0173d634826b998bdc304f93c0966b8)
> > | | Merge: 2c1e6a3 33e70e7
> > | | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
> > | | Date: Tue Feb 5 22:05:33 2013 +0100
> >
> > The second "commit" line (diff with second parent) doesn't have the
> > "| |" prefix, I don't think this is intentional.
>
> The second "commit" line should start with "| * ":
No. That would indicate a commit on the branch that is the second
parent of the first commit. But this is the same commit as the one
above, just with a diff against its second parent instead of its first
parent.
I would argue that the line should start with "| | ", since it really is
just a continuation of the same commit.
| |
| | commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 33e70e70c0173d634826b998bdc304f93c0966b8)
| | Merge: 2c1e6a3 33e70e7
| | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
| | Date: Tue Feb 5 22:05:33 2013 +0100
John
^ permalink raw reply
* Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Junio C Hamano @ 2013-02-06 15:47 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jiang Xin, Git List
In-Reply-To: <CACsJy8DcXuFqjBtufQq1-0Vm3H4uxs03Crx+akY-kbrVgY8vUw@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> How about utf8_fwprintf? wprintf() deals with wide characters and
> returns the number of wide characters, I think the name fits. And we
> could just drop utf8_ and use the existing name, because we don't use
> wchar_t* anyway.
Please, no. That line of reasoning shows a horrible design taste
(or lack of taste). "We don't use X right now" (or "We will promise
never to use X", for that matter) is never a good reason to abuse a
name that normal people would closely associate with X to something
that is completely different. That leads to more confusion, not
less.
I guess utf8_fprintf() is not so bad after all. fprintf() without
the utf8_ prefix is perfectly capable of showing a string encoded in
UTF-8, and anybody can correctly guess that the magic utf8_ prefix
would introduce (i.e. the difference between utf8_fprintf and
fprintf) can only be about the return value. It can be reasonably
expected that everybody would then know that the display column
count can be the only sane return value that is different from what
fprintf() would return.
^ permalink raw reply
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Junio C Hamano @ 2013-02-06 15:56 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Schubert, Shawn Pearce, git
In-Reply-To: <20130206103952.GA5267@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Is it worth having a strbuf_set* family of functions to match the
> strbuf_add*? We seem to have these sorts of errors with strbuf from time
> to time, and I wonder if that would make it easier (and more readable)
> to do the right thing.
Possibly.
The callsite below may be a poor example, though; you would need the
_reset() even if you change the _addstr() we can see in the context
to _setstr() to make sure later strbuf_*(type) will start from a
clean slate when !t anyway, no?
>
> http.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/http.c b/http.c
> index d868d8b..d9d1aad 100644
> --- a/http.c
> +++ b/http.c
> @@ -841,6 +841,7 @@ static int http_request(const char *url, struct strbuf *type,
>
> if (type) {
> char *t;
> + strbuf_reset(type);
> curl_easy_getinfo(slot->curl, CURLINFO_CONTENT_TYPE, &t);
> if (t)
> strbuf_addstr(type, t);
^ permalink raw reply
* Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies
From: Junio C Hamano @ 2013-02-06 15:57 UTC (permalink / raw)
To: Jeff King; +Cc: git, Shawn Pearce
In-Reply-To: <20130206113112.GB5267@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Feb 05, 2013 at 07:45:01AM -0800, Junio C Hamano wrote:
>
>> > In the earlier review, I mentioned making this per-service, but I see
>> > that is not the case here. Do you have an argument against doing so?
>>
>> Perhaps then I misunderstood your intention. By reminding me of the
>> receive-pack side, I thought you were hinting to unify these two
>> into one, which I did. There is no argument against it.
>
> What I meant was that there should be transfer.hiderefs, and an
> individual {receive,uploadpack}.hiderefs, similar to the way we have
> transfer.unpacklimit.
Yes, as I said, I misunderstood your intention.
^ permalink raw reply
* How to diff 2 file revisions with gitk
From: R. Diez @ 2013-02-06 15:57 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <1360164008.49200.YahooMailNeo@web171204.mail.ir2.yahoo.com>
Hi there:
I asked a few days ago whether I could easily diff 2 file revisions with the mouse in gitk, but I got no reply yet, see here:
How to diff two file revisions with the mouse (with gitk)
https://groups.google.com/forum/#!topic/git-users/9znsQsTB0dE
I am hoping that it was the wrong mailing list, and this one the right one. 8-)
Here is the full question text again:
--------8<--------8<--------8<--------8<--------
I would like to start gitk, select with the mouse 2
revisions of some file and then compare them, hopefully with an external
diff tool, very much like I am used to with WinCVS.
The closest I
got is to start gitk with a filename as an argument, in order to
restrict the log to that one file. Then I right-click on a commit (a
file revision) and choose "Mark this commit". However, if I right-click
on another commit and choose "Compare with marked commit", I get a full
commit diff with all files, and not just the file I specified on the
command-line arguments.
Selecting a filename in the "Tree" view and choosing "Highlight this only", as I found on the Internet, does not seem to help.
I have git 1.7.9 (on Cygwin). Can someone help?
By the way, it would be nice if gitk could launch the external diff tool from the "Compare with marked commit" option too.
--------8<--------8<--------8<--------8<--------
Thanks in advance,
rdiez
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-02-06 15:58 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, Jeff King, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <vpqmwvhxyuj.fsf@grenoble-inp.fr>
On Wed, 06 Feb 2013 16:10:12 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
MM> Ted Zlatanov <tzz@lifelogs.com> writes:
MM> [...] so the way to go for send-email is probably to libify the
MM> credential support in git-remote-mediawiki, and to use it in send-email.
>>
>> I looked and that's indeed very useful. If it's put in a library, I'd
>> use credential_read() and credential_write() in my netrc credential
>> helper. But I would formalize it a little more about the token names
>> and output,
MM> Can you elaborate on this? The idea of the Perl code was to mimick a
MM> call to the C API, keeping essentially the same names.
None of these are a big deal, and Michal said he's working on libifying
this anyhow:
- making 'fill' a special operation is weird
- anchor the key regex to beginning of line (not strictly necessary)
- sort the output tokens (after 'url' is extracted) so the output is consistent and testable
>> and I wouldn't necessarily die() on error.
MM> Sure, die()ing in a library is bad.
>> Maybe this can be merged with the netrc credential helper's
>> read_credential_data_from_stdin() and print_credential_data()?
MM> I don't know about the netrc credential helper, but I guess that's
MM> another layer. The git-remote-mediawiki code is the code to call the
MM> credential C API, that in turn may (or may not) call a credential
MM> helper.
Yup. But what you call "read" and "write" are, to the credential
helper, "write" and "read" but it's the same protocol :) So maybe the
names should be changed to reflect that, e.g. "query" and "response."
MM> One thing to be careful about: git-remote-mediawiki is currently a
MM> standalone script, so it can be installed with a plain "cp
MM> git-remote-mediawiki $somewhere/". One consequence of libification
MM> is that it adds a dependency on the library (e.g. Git.pm). We should
MM> be carefull to keep it easy for the user to install it (e.g. some
MM> kind of "make install", or update the doc).
I don't know--it's up to the `git-remote-mediawiki' maintainers... But
I think anywhere you have Git, you also have Git.pm, right? Maybe? But
then you also have to look at whether Git.pm has the functionality you
need... so I better go quiet :)
Ted
^ permalink raw reply
* [Request] Git export with hardlinks
From: Thomas Koch @ 2013-02-06 15:19 UTC (permalink / raw)
To: git
Hi,
I'd like to script a git export command that can be given a list of already
exported worktrees and the tree SHA1s these worktrees correspond too. The git
export command should then for every file it wants to export lookup in the
existing worktrees whether an identical file is already present and in that
case hardlink to the new export location instead of writing the same file
again.
Use Case: A git based web deployment system that exports git trees to be
served by a web server. Every new deployment is written to a new folder. After
the export the web server should start serving new requests from the new
folder.
It might be possible that this is premature optimization. But I'd like to
learn more Python and dulwich by hacking this.
Do you have any additional thoughts or use cases about this?
Regards,
Thomas Koch, http://www.koch.ro
^ permalink raw reply
* Re: CodingGuidelines Perl amendment
From: Junio C Hamano @ 2013-02-06 16:29 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: git, Jeff King
In-Reply-To: <871ucto4vj.fsf_-_@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> - As in C (see above), we avoid using braces unnecessarily (but Perl
> forces braces around if/unless/else/foreach blocks, so this is not
> always possible).
Is it ever (as opposed to "not always") possible to omit braces?
It sounds as if we encourage the use of statement modifiers, which
certainly is not what I want to see.
You probably would want to mention that opening braces for
"if/else/elsif" do not sit on their own line, and closing braces for
them will be followed the next "else/elseif" on the same line
instead, but that is part of "most of the C guidelines above apply"
so it may be redundant.
> - Don't abuse statement modifiers (unless $youmust).
It does not make a useful guidance to leave $youmust part
unspecified.
Incidentally, your sentence is a good example of where use of
statement modifiers is appropriate: $youmust is rarely true.
In general:
... do something ...
do_this() unless (condition);
... do something else ...
is easier to follow the flow of the logic than
... do something ...
unless (condition) {
do_this();
}
... do something else ...
*only* when condition is extremely rare, iow, when do_this() is
expected to be almost always called.
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Matthieu Moy @ 2013-02-06 16:41 UTC (permalink / raw)
To: Ted Zlatanov
Cc: Junio C Hamano, Jeff King, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <87sj59mo2y.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> None of these are a big deal, and Michal said he's working on libifying
> this anyhow:
>
> - making 'fill' a special operation is weird
Well, 'fill' is the only operation that mutates the credential structure
(i.e. the only one for which "git credential" emits an output to be
parsed), so you don't have much choice.
> - anchor the key regex to beginning of line (not strictly necessary)
Right. The greedyness of * ensures correction, but I like explicit
anchors ^...$ too.
> - sort the output tokens (after 'url' is extracted) so the output is consistent and testable
Why not, if you want to use the output of credential_write in tests. But
credential_write is essentially used to talk to "git credential", so the
important information is the content of the hash before credential_write
and after credential_read. They are unordered, but consistent and
testable.
>>> Maybe this can be merged with the netrc credential helper's
>>> read_credential_data_from_stdin() and print_credential_data()?
>
> MM> I don't know about the netrc credential helper, but I guess that's
> MM> another layer. The git-remote-mediawiki code is the code to call the
> MM> credential C API, that in turn may (or may not) call a credential
> MM> helper.
>
> Yup. But what you call "read" and "write" are, to the credential
> helper, "write" and "read" but it's the same protocol :) So maybe the
> names should be changed to reflect that, e.g. "query" and "response."
I don't think that would be a better naming. Maybe "serialize" and
"parse" would be better, but "query" would sound like it establishes the
connection and possibly reads the response to me.
> MM> One thing to be careful about: git-remote-mediawiki is currently a
> MM> standalone script, so it can be installed with a plain "cp
> MM> git-remote-mediawiki $somewhere/". One consequence of libification
> MM> is that it adds a dependency on the library (e.g. Git.pm). We should
> MM> be carefull to keep it easy for the user to install it (e.g. some
> MM> kind of "make install", or update the doc).
>
> I don't know--it's up to the `git-remote-mediawiki' maintainers...
That is, me ;-).
> But I think anywhere you have Git, you also have Git.pm, right?
Yes, but you have to find out where it is installed. Git's Makefile
hardcodes the path to Git.pm at build time, inserting one line in the
perl script:
use lib (split(/:/, $ENV{GITPERLLIB} || "$INSTLIBDIR"));
The same needs to be done for git-remote-mediawiki. As much as possible,
I'd rather avoid copy-pasting from Git's Makefile, so this means
extracting the perl part of Git's Makefile and make it available in
contrib/.
I'll try a patch in this direction.
> Maybe? But then you also have to look at whether Git.pm has the
> functionality you need...
If git-remote-mediawiki is installed from Git's source, I think it's OK
to assume that Git.pm will be up to date, but that would be even better
if we can issue a clean error message when the functions to be called do
not exist.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [RFC/PATCH 2/4] cat-file: do not die on --textconv without textconv filters
From: Junio C Hamano @ 2013-02-06 16:47 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <b20e91bc71e59b5390005f2e6428e69a467e80b5.1360162813.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> When a command is supposed to use textconv filters (by default or with
> "--textconv") and none are configured then the blob is output without
> conversion; the only exception to this rule is "cat-file --textconv".
I am of two minds. Because cat-file is mostly a low-level plumbing,
I do not necessarily think it is a bad behaviour for it to error out
when it was asked to apply textconv where there is no filter or when
the filter fails to produce an output. On the other hand, it
certainly makes it more convenient for callers that do not care too
deeply, taking textconv as a mere hint just like Porcelains do.
>
> Make it behave like the rest of textconv aware commands.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> builtin/cat-file.c | 9 +++++----
> t/t8007-cat-file-textconv.sh | 20 +++++---------------
> 2 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 00528dd..6912dc2 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -146,10 +146,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
> die("git cat-file --textconv %s: <object> must be <sha1:path>",
> obj_name);
>
> - if (!textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
> - die("git cat-file --textconv: unable to run textconv on %s",
> - obj_name);
> - break;
> + if (textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
> + break;
> +
> + /* otherwise expect a blob */
> + exp_type = "blob";
Please use the constant string blob_type that is available for all
callers including this one.
But stepping back a bit.
What happens when I say "cat-file -c HEAD:Documentation", and what
should happen when I do so?
I think what we want to see in the ideal world might be:
* If we have a textconv for tree objects at that path to format it
specially, textconv_object() may be allowed to textualize it
(even though it is not a blob, and textconv so far has always
been about blobs; it needs to be considered carefully if it makes
sense to allow such a usage) and show it;
* If we don't, we act as if -c were -p; in other words, we treat
the built-in "human output" implemented by "cat-file -p" as if
that is a textconv.
In other words, you may want to fall-thru to case 'p', not case 0
with forced "blob" type.
^ permalink raw reply
* Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Junio C Hamano @ 2013-02-06 16:53 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <883f0163cb732932061a368ea9bc187c13e4ecca.1360162813.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Currently, "diff" and "cat-file" for blobs obey "--textconv" options
> (with the former defaulting to "--textconv" and the latter to
> "--no-textconv") whereas "show" does not obey this option, even though
> it takes diff options.
>
> Make "show" on blobs behave like "diff", i.e. obey "--textconv" by
> default and "--no-textconv" when given.
What does "log -p" do currently, and what should it do? Does/should
it also use --textconv?
The --textconv is a natural extension of what --ext-diff provides us,
so I think it should trigger the same way as how --ext-diff triggers.
We apply "--ext-diff" for "diff" by default but not for "log -p" and
"show"; I suspect this may have been for a good reason but I do not
recall the discussion that led to the current behaviour offhand.
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> builtin/log.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 8f0b2e8..f83870d 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -402,10 +402,28 @@ static void show_tagger(char *buf, int len, struct rev_info *rev)
> strbuf_release(&out);
> }
>
> -static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
> +static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, const char *obj_name)
> {
> + unsigned char sha1c[20];
> + struct object_context obj_context;
> + char *buf;
> + unsigned long size;
> +
> fflush(stdout);
> - return stream_blob_to_fd(1, sha1, NULL, 0);
> + if (!DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
> + return stream_blob_to_fd(1, sha1, NULL, 0);
> +
> + if (get_sha1_with_context(obj_name, 0, sha1c, &obj_context))
> + die("Not a valid object name %s", obj_name);
> + if (!obj_context.path[0] ||
> + !textconv_object(obj_context.path, obj_context.mode, sha1c, 1, &buf, &size))
> + return stream_blob_to_fd(1, sha1, NULL, 0);
> +
> + if (!buf)
> + die("git show %s: bad file", obj_name);
> +
> + write_or_die(1, buf, size);
> + return 0;
> }
>
> static int show_tag_object(const unsigned char *sha1, struct rev_info *rev)
> @@ -491,7 +509,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
> const char *name = objects[i].name;
> switch (o->type) {
> case OBJ_BLOB:
> - ret = show_blob_object(o->sha1, NULL);
> + ret = show_blob_object(o->sha1, &rev, name);
> break;
> case OBJ_TAG: {
> struct tag *t = (struct tag *)o;
^ permalink raw reply
* Re: [RFC/PATCH 0/4] textconv for show and grep
From: Junio C Hamano @ 2013-02-06 16:55 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
The parts for "grep" in the series makes tons of sense to me. I am
not yet convinced about the other two, though.
Thanks.
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-02-06 17:40 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, Jeff King, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <vpqobfxwg2q.fsf@grenoble-inp.fr>
On Wed, 06 Feb 2013 17:41:01 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
MM> Ted Zlatanov <tzz@lifelogs.com> writes:
>> - sort the output tokens (after 'url' is extracted) so the output is consistent and testable
MM> Why not, if you want to use the output of credential_write in tests. But
MM> credential_write is essentially used to talk to "git credential", so the
MM> important information is the content of the hash before credential_write
MM> and after credential_read. They are unordered, but consistent and
MM> testable.
I like testing output (especially when it's part of an API), so we
should make the externally observable output consistent and testable.
The change is tiny, just sort the keys instead of calling each(), so I
hope it makes it in the final version.
>> Yup. But what you call "read" and "write" are, to the credential
>> helper, "write" and "read" but it's the same protocol :) So maybe the
>> names should be changed to reflect that, e.g. "query" and "response."
MM> I don't think that would be a better naming. Maybe "serialize" and
MM> "parse" would be better, but "query" would sound like it establishes the
MM> connection and possibly reads the response to me.
I'm OK with anything unambiguous.
Thanks!
Ted
^ permalink raw reply
* Re: CodingGuidelines Perl amendment
From: demerphq @ 2013-02-06 17:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ted Zlatanov, git, Jeff King
In-Reply-To: <7vvca5mmmt.fsf@alter.siamese.dyndns.org>
On 6 February 2013 17:29, Junio C Hamano <gitster@pobox.com> wrote:
> Ted Zlatanov <tzz@lifelogs.com> writes:
>
>> - As in C (see above), we avoid using braces unnecessarily (but Perl
>> forces braces around if/unless/else/foreach blocks, so this is not
>> always possible).
>
> Is it ever (as opposed to "not always") possible to omit braces?
Only in a statement modifier.
> It sounds as if we encourage the use of statement modifiers, which
> certainly is not what I want to see.
As you mention below statement modifiers have their place. For instance
next if $whatever;
Is considered preferable to
if ($whatever) {
next;
}
Similarly
open my $fh, ">", $filename
or die "Failed to open '$filename': $!";
Is considered preferable by most Perl programmers to:
my $fh;
if ( not open $fh, ">", $filename ) {
die "Failed to open '$filename': $!";
}
> You probably would want to mention that opening braces for
> "if/else/elsif" do not sit on their own line,
> and closing braces for
> them will be followed the next "else/elseif" on the same line
> instead, but that is part of "most of the C guidelines above apply"
> so it may be redundant.
>
>> - Don't abuse statement modifiers (unless $youmust).
>
> It does not make a useful guidance to leave $youmust part
> unspecified.
>
> Incidentally, your sentence is a good example of where use of
> statement modifiers is appropriate: $youmust is rarely true.
"unless" often leads to maintenance errors as the expression gets more
complicated over time, more branches need to be added to the
statement, etc. Basically people are bad at doing De Morgans law in
their head.
> In general:
>
> ... do something ...
> do_this() unless (condition);
> ... do something else ...
>
> is easier to follow the flow of the logic than
>
> ... do something ...
> unless (condition) {
> do_this();
> }
> ... do something else ...
>
> *only* when condition is extremely rare, iow, when do_this() is
> expected to be almost always called.
if (not $condition) {
do_this();
}
Is much less error prone in terms of maintenance than
unless ($condition) {
do_this();
}
Similarly
do_this() if not $condition;
leads to less maintenance errors than
do_this() unless $condition;
So if you objective is maintainability I would just ban "unless" outright.
Cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
* [PATCH] Update CodingGuidelines for Perl 5
From: Ted Zlatanov @ 2013-02-06 17:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vvca5mmmt.fsf@alter.siamese.dyndns.org>
Update the coding guidelines for Perl 5.
Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
Documentation/CodingGuidelines | 44 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 1d7de5f..951d74c 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -179,6 +179,50 @@ For C programs:
- Use Git's gettext wrappers to make the user interface
translatable. See "Marking strings for translation" in po/README.
+For Perl 5 programs:
+
+ - Most of the C guidelines above apply.
+
+ - We try to support Perl 5.8 and later ("use Perl 5.008").
+
+ - use strict and use warnings are strongly preferred.
+
+ - As in C (see above), we avoid using braces unnecessarily (but Perl forces
+ braces around if/unless/else/foreach blocks, so this is not always possible).
+ At least make sure braces do not sit on their own line, like with C.
+
+ - Don't abuse statement modifiers--they are discouraged. But in general:
+
+ ... do something ...
+ do_this() unless (condition);
+ ... do something else ...
+
+ should be used instead of
+
+ ... do something ...
+ unless (condition) {
+ do_this();
+ }
+ ... do something else ...
+
+ *only* when when the condition is so rare that do_this() will be called
+ almost always.
+
+ - We try to avoid assignments inside if().
+
+ - Learn and use Git.pm if you need that functionality.
+
+ - For Emacs, it's useful to put the following in
+ GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:
+
+ ;; note the first part is useful for C editing, too
+ ((nil . ((indent-tabs-mode . t)
+ (tab-width . 8)
+ (fill-column . 80)))
+ (cperl-mode . ((cperl-indent-level . 8)
+ (cperl-extra-newline-before-brace . nil)
+ (cperl-merge-trailing-else . t))))
+
Writing Documentation:
Every user-visible change should be reflected in the documentation.
--
1.7.9.rc2
^ permalink raw reply related
* Re: CodingGuidelines Perl amendment
From: Ted Zlatanov @ 2013-02-06 18:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vvca5mmmt.fsf@alter.siamese.dyndns.org>
On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Is it ever (as opposed to "not always") possible to omit braces?
Oh yes! Not that I recommend it, and I'm not even going to touch on
Perl Golf :)
JCH> It sounds as if we encourage the use of statement modifiers, which
JCH> certainly is not what I want to see.
Yup. I think I captured that in the patch, but please feel free to
revise it after applying or throw it back to me.
JCH> You probably would want to mention that opening braces for
JCH> "if/else/elsif" do not sit on their own line, and closing braces for
JCH> them will be followed the next "else/elseif" on the same line
JCH> instead, but that is part of "most of the C guidelines above apply"
JCH> so it may be redundant.
OK; done.
>> - Don't abuse statement modifiers (unless $youmust).
JCH> It does not make a useful guidance to leave $youmust part
JCH> unspecified.
JCH> Incidentally, your sentence is a good example of where use of
JCH> statement modifiers is appropriate: $youmust is rarely true.
I was trying to be funny, honestly. But OK; reworded.
Ted
^ permalink raw reply
* Re: CodingGuidelines Perl amendment
From: Ted Zlatanov @ 2013-02-06 18:08 UTC (permalink / raw)
To: demerphq; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <CANgJU+V5bhdpN_kWxQPEJgx24LXLtQJWRbnHwkSgm9zFwzm+fA@mail.gmail.com>
On Wed, 6 Feb 2013 18:45:56 +0100 demerphq <demerphq@gmail.com> wrote:
d> So if you objective is maintainability I would just ban "unless" outright.
Please consider me opposed to such a ban.
Ted
^ permalink raw reply
* Re: How to diff 2 file revisions with gitk
From: Johannes Sixt @ 2013-02-06 18:09 UTC (permalink / raw)
To: R. Diez; +Cc: git@vger.kernel.org
In-Reply-To: <1360166273.33888.YahooMailNeo@web171204.mail.ir2.yahoo.com>
Am 06.02.2013 16:57, schrieb R. Diez:
> I would like to start gitk, select with the mouse 2
> revisions of some file and then compare them, hopefully with an external
> diff tool, very much like I am used to with WinCVS.
>
> The closest I
> got is to start gitk with a filename as an argument, in order to
> restrict the log to that one file. Then I right-click on a commit (a
> file revision) and choose "Mark this commit". However, if I right-click
> on another commit and choose "Compare with marked commit", I get a full
> commit diff with all files, and not just the file I specified on the
> command-line arguments.
Edit->Preferences, tick 'Limit diff to listed paths'.
-- Hannes
^ permalink raw reply
* [PATCH 2/4] perl.mak: introduce $(GIT_ROOT_DIR) to allow inclusion from other directories
From: Matthieu Moy @ 2013-02-06 18:11 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr>
perl.mak uses relative path, which is OK when called from the toplevel,
but won't be anymore if one includes it from elsewhere. It is now
possible to include the file using:
GIT_ROOT_DIR=<whatever>
include $(GIT_ROOT_DIR)/perl.mak
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
perl.mak | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/perl.mak b/perl.mak
index 8bbeef3..a2b8717 100644
--- a/perl.mak
+++ b/perl.mak
@@ -1,5 +1,9 @@
# Rules to build Git commands written in perl
+ifndef GIT_ROOT_DIR
+ GIT_ROOT_DIR = .
+endif
+
ifndef PERL_PATH
PERL_PATH = /usr/bin/perl
endif
@@ -11,12 +15,11 @@ NO_PERL = NoThanks
endif
ifndef NO_PERL
-$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
-
+$(patsubst %.perl,%,$(SCRIPT_PERL)): $(GIT_ROOT_DIR)/perl/perl.mak
-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl $(GIT_ROOT_DIR)/GIT-VERSION-FILE
$(QUIET_GEN)$(RM) $@ $@+ && \
- INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
+ INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C $(GIT_ROOT_DIR)/perl -s --no-print-directory instlibdir` && \
sed -e '1{' \
-e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \
-e ' h' \
--
1.8.1.2.526.gf51a757
^ permalink raw reply related
* [PATCH 3/4] Makefile: factor common configuration in git-default-config.mak
From: Matthieu Moy @ 2013-02-06 18:11 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr>
Similarly to the extraction of perl-related code in perl.mak, we extract
general default configuration from the Makefile to make it available from
directories other than the toplevel.
This is required to make perl.mak usable because it requires $(pathsep)
to be set.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Makefile | 62 +-----------------------------------------------------
default-config.mak | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 61 deletions(-)
create mode 100644 default-config.mak
diff --git a/Makefile b/Makefile
index f39d4a9..9649a41 100644
--- a/Makefile
+++ b/Makefile
@@ -346,67 +346,7 @@ GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
-include GIT-VERSION-FILE
-# CFLAGS and LDFLAGS are for the users to override from the command line.
-
-CFLAGS = -g -O2 -Wall
-LDFLAGS =
-ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
-ALL_LDFLAGS = $(LDFLAGS)
-STRIP ?= strip
-
-# Among the variables below, these:
-# gitexecdir
-# template_dir
-# mandir
-# infodir
-# htmldir
-# sysconfdir
-# can be specified as a relative path some/where/else;
-# this is interpreted as relative to $(prefix) and "git" at
-# runtime figures out where they are based on the path to the executable.
-# This can help installing the suite in a relocatable way.
-
-prefix = $(HOME)
-bindir_relative = bin
-bindir = $(prefix)/$(bindir_relative)
-mandir = share/man
-infodir = share/info
-gitexecdir = libexec/git-core
-mergetoolsdir = $(gitexecdir)/mergetools
-sharedir = $(prefix)/share
-gitwebdir = $(sharedir)/gitweb
-localedir = $(sharedir)/locale
-template_dir = share/git-core/templates
-htmldir = share/doc/git-doc
-ETC_GITCONFIG = $(sysconfdir)/gitconfig
-ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
-lib = lib
-# DESTDIR =
-pathsep = :
-
-export prefix bindir sharedir sysconfdir gitwebdir localedir
-
-CC = cc
-AR = ar
-RM = rm -f
-DIFF = diff
-TAR = tar
-FIND = find
-INSTALL = install
-RPMBUILD = rpmbuild
-TCL_PATH = tclsh
-TCLTK_PATH = wish
-XGETTEXT = xgettext
-MSGFMT = msgfmt
-PTHREAD_LIBS = -lpthread
-PTHREAD_CFLAGS =
-GCOV = gcov
-
-export TCL_PATH TCLTK_PATH
-
-SPARSE_FLAGS =
-
-
+include default-config.mak
### --- END CONFIGURATION SECTION ---
diff --git a/default-config.mak b/default-config.mak
new file mode 100644
index 0000000..b2aab3d
--- /dev/null
+++ b/default-config.mak
@@ -0,0 +1,61 @@
+# CFLAGS and LDFLAGS are for the users to override from the command line.
+
+CFLAGS = -g -O2 -Wall
+LDFLAGS =
+ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
+ALL_LDFLAGS = $(LDFLAGS)
+STRIP ?= strip
+
+# Among the variables below, these:
+# gitexecdir
+# template_dir
+# mandir
+# infodir
+# htmldir
+# sysconfdir
+# can be specified as a relative path some/where/else;
+# this is interpreted as relative to $(prefix) and "git" at
+# runtime figures out where they are based on the path to the executable.
+# This can help installing the suite in a relocatable way.
+
+prefix = $(HOME)
+bindir_relative = bin
+bindir = $(prefix)/$(bindir_relative)
+mandir = share/man
+infodir = share/info
+gitexecdir = libexec/git-core
+mergetoolsdir = $(gitexecdir)/mergetools
+sharedir = $(prefix)/share
+gitwebdir = $(sharedir)/gitweb
+localedir = $(sharedir)/locale
+template_dir = share/git-core/templates
+htmldir = share/doc/git-doc
+ETC_GITCONFIG = $(sysconfdir)/gitconfig
+ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
+lib = lib
+# DESTDIR =
+pathsep = :
+
+export prefix bindir sharedir sysconfdir gitwebdir localedir
+
+CC = cc
+AR = ar
+RM = rm -f
+DIFF = diff
+TAR = tar
+FIND = find
+INSTALL = install
+RPMBUILD = rpmbuild
+TCL_PATH = tclsh
+TCLTK_PATH = wish
+XGETTEXT = xgettext
+MSGFMT = msgfmt
+PTHREAD_LIBS = -lpthread
+PTHREAD_CFLAGS =
+GCOV = gcov
+
+export TCL_PATH TCLTK_PATH
+
+SPARSE_FLAGS =
+
+
--
1.8.1.2.526.gf51a757
^ permalink raw reply related
* [PATCH 1/4] Makefile: extract perl-related rules to make them available from other dirs
From: Matthieu Moy @ 2013-02-06 18:11 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr>
The final goal is to make it easy to write Git commands in perl in the
contrib/ directory. It is currently possible to do so, but without the
benefits of Git's Makefile: adapt first line with $(PERL_PATH),
hardcode the path to Git.pm, ...
We make the perl-related part of the Makefile available from directories
other than the toplevel so that:
* Developers can include it, to avoid code duplication
* Users can get a consistent behavior of "make install"
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Makefile | 46 +---------------------------------------------
perl.mak | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 45 deletions(-)
create mode 100644 perl.mak
diff --git a/Makefile b/Makefile
index 731b6a8..f39d4a9 100644
--- a/Makefile
+++ b/Makefile
@@ -573,14 +573,10 @@ BINDIR_PROGRAMS_NO_X += git-cvsserver
ifndef SHELL_PATH
SHELL_PATH = /bin/sh
endif
-ifndef PERL_PATH
- PERL_PATH = /usr/bin/perl
-endif
ifndef PYTHON_PATH
PYTHON_PATH = /usr/bin/python
endif
-export PERL_PATH
export PYTHON_PATH
LIB_FILE = libgit.a
@@ -1441,10 +1437,6 @@ ifeq ($(TCLTK_PATH),)
NO_TCLTK = NoThanks
endif
-ifeq ($(PERL_PATH),)
-NO_PERL = NoThanks
-endif
-
ifeq ($(PYTHON_PATH),)
NO_PYTHON = NoThanks
endif
@@ -1522,7 +1514,6 @@ prefix_SQ = $(subst ','\'',$(prefix))
gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
DIFF_SQ = $(subst ','\'',$(DIFF))
@@ -1715,9 +1706,6 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script) && \
mv $@+ $@
-ifndef NO_PERL
-$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
-
perl/perl.mak: perl/PM.stamp
perl/PM.stamp: FORCE
@@ -1728,39 +1716,7 @@ perl/PM.stamp: FORCE
perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
- $(QUIET_GEN)$(RM) $@ $@+ && \
- INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
- sed -e '1{' \
- -e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \
- -e ' h' \
- -e ' s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
- -e ' H' \
- -e ' x' \
- -e '}' \
- -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
- $@.perl >$@+ && \
- chmod +x $@+ && \
- mv $@+ $@
-
-
-.PHONY: gitweb
-gitweb:
- $(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
-
-git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES
- $(QUIET_GEN)$(cmd_munge_script) && \
- chmod +x $@+ && \
- mv $@+ $@
-else # NO_PERL
-$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
- $(QUIET_GEN)$(RM) $@ $@+ && \
- sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
- -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
- unimplemented.sh >$@+ && \
- chmod +x $@+ && \
- mv $@+ $@
-endif # NO_PERL
+include perl.mak
ifndef NO_PYTHON
$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
diff --git a/perl.mak b/perl.mak
new file mode 100644
index 0000000..8bbeef3
--- /dev/null
+++ b/perl.mak
@@ -0,0 +1,49 @@
+# Rules to build Git commands written in perl
+
+ifndef PERL_PATH
+ PERL_PATH = /usr/bin/perl
+endif
+export PERL_PATH
+PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+
+ifeq ($(PERL_PATH),)
+NO_PERL = NoThanks
+endif
+
+ifndef NO_PERL
+$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
+
+
+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
+ $(QUIET_GEN)$(RM) $@ $@+ && \
+ INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
+ sed -e '1{' \
+ -e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \
+ -e ' h' \
+ -e ' s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
+ -e ' H' \
+ -e ' x' \
+ -e '}' \
+ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+ $@.perl >$@+ && \
+ chmod +x $@+ && \
+ mv $@+ $@
+
+
+.PHONY: gitweb
+gitweb:
+ $(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
+
+git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES
+ $(QUIET_GEN)$(cmd_munge_script) && \
+ chmod +x $@+ && \
+ mv $@+ $@
+else # NO_PERL
+$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
+ $(QUIET_GEN)$(RM) $@ $@+ && \
+ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+ -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
+ unimplemented.sh >$@+ && \
+ chmod +x $@+ && \
+ mv $@+ $@
+endif # NO_PERL
--
1.8.1.2.526.gf51a757
^ permalink raw reply related
* [PATCH 0/4] Allow contrib/ to use Git's Makefile for perl code
From: Matthieu Moy @ 2013-02-06 18:11 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <vpqobfxwg2q.fsf@grenoble-inp.fr>
The very final goal is to be able to move painlessly (credential) code
from git-remote-mediawiki to Git.pm, but then it's nice for the user
to be able to say just "cd contrib/mw-to-git && make install" and let
the Makefile set perl's library path just like other Git commands
written in perl.
This series does this while trying to minimize code duplication, and
to make it easy for future other tools in contrib to do the same.
Matthieu Moy (4):
Makefile: extract perl-related rules to make them available from other
dirs
perl.mak: introduce $(GIT_ROOT_DIR) to allow inclusion from other
directories
Makefile: factor common configuration in git-default-config.mak
git-remote-mediawiki: use Git's Makefile to build the script
Makefile | 108 +--------------------
contrib/mw-to-git/.gitignore | 1 +
contrib/mw-to-git/Makefile | 45 ++++++---
...-remote-mediawiki => git-remote-mediawiki.perl} | 0
default-config.mak | 61 ++++++++++++
perl.mak | 52 ++++++++++
6 files changed, 145 insertions(+), 122 deletions(-)
create mode 100644 contrib/mw-to-git/.gitignore
rename contrib/mw-to-git/{git-remote-mediawiki => git-remote-mediawiki.perl} (100%)
create mode 100644 default-config.mak
create mode 100644 perl.mak
--
1.8.1.2.526.gf51a757
^ permalink raw reply
* [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script
From: Matthieu Moy @ 2013-02-06 18:11 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr>
The configuration of the install directory is not reused from the
toplevel Makefile: we assume Git is already built, hence just call
"git --exec-path". This avoids too much surgery in the toplevel Makefile.
git-remote-mediawiki.perl can now "use Git;".
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
contrib/mw-to-git/.gitignore | 1 +
contrib/mw-to-git/Makefile | 45 ++++++++++++++--------
...-remote-mediawiki => git-remote-mediawiki.perl} | 0
3 files changed, 30 insertions(+), 16 deletions(-)
create mode 100644 contrib/mw-to-git/.gitignore
rename contrib/mw-to-git/{git-remote-mediawiki => git-remote-mediawiki.perl} (100%)
diff --git a/contrib/mw-to-git/.gitignore b/contrib/mw-to-git/.gitignore
new file mode 100644
index 0000000..b919655
--- /dev/null
+++ b/contrib/mw-to-git/.gitignore
@@ -0,0 +1 @@
+git-remote-mediawiki
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index 3ed728b..ed8073b 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -8,40 +8,53 @@
#
## Build git-remote-mediawiki
--include ../../config.mak.autogen
--include ../../config.mak
+all:
+
+GIT_ROOT_DIR=../../
+include $(GIT_ROOT_DIR)/default-config.mak
+-include $(GIT_ROOT_DIR)/config.mak.autogen
+-include $(GIT_ROOT_DIR)/config.mak
+-include $(GIT_ROOT_DIR)/GIT-VERSION-FILE
+
+
+SCRIPT_PERL = git-remote-mediawiki.perl
+ALL_PROGRAMS = $(patsubst %.perl,%,$(SCRIPT_PERL))
+
+include $(GIT_ROOT_DIR)/perl.mak
-ifndef PERL_PATH
- PERL_PATH = /usr/bin/perl
-endif
ifndef gitexecdir
gitexecdir = $(shell git --exec-path)
endif
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
-gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
-SCRIPT = git-remote-mediawiki
+ifneq ($(filter /%,$(firstword $(gitexecdir))),)
+gitexec_instdir = $(gitexecdir)
+else
+gitexec_instdir = $(prefix)/$(gitexecdir)
+endif
+gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
.PHONY: install help doc test clean
help:
@echo 'This is the help target of the Makefile. Current configuration:'
- @echo ' gitexecdir = $(gitexecdir_SQ)'
+ @echo ' gitexec_instdir = $(gitexec_instdir_SQ)'
@echo ' PERL_PATH = $(PERL_PATH_SQ)'
- @echo 'Run "$(MAKE) install" to install $(SCRIPT) in gitexecdir'
+ @echo 'Run "$(MAKE) all" to build the script'
+ @echo 'Run "$(MAKE) install" to install $(ALL_PROGRAMS) in gitexec_instdir'
@echo 'Run "$(MAKE) test" to run the testsuite'
-install:
- sed -e '1s|#!.*/perl|#!$(PERL_PATH_SQ)|' $(SCRIPT) \
- > '$(gitexecdir_SQ)/$(SCRIPT)'
- chmod +x '$(gitexecdir)/$(SCRIPT)'
+all: $(ALL_PROGRAMS)
+
+install: $(ALL_PROGRAMS)
+ $(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
doc:
- @echo 'Sorry, "make doc" is not implemented yet for $(SCRIPT)'
+ @echo 'Sorry, "make doc" is not implemented yet for $(ALL_PROGRAMS)'
test:
$(MAKE) -C t/ test
clean:
- $(RM) '$(gitexecdir)/$(SCRIPT)'
+ $(RM) $(ALL_PROGRAMS)
+ $(RM) $(patsubst %,$(gitexec_instdir)/%,/$(ALL_PROGRAMS))
$(MAKE) -C t/ clean
diff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki.perl
similarity index 100%
rename from contrib/mw-to-git/git-remote-mediawiki
rename to contrib/mw-to-git/git-remote-mediawiki.perl
--
1.8.1.2.526.gf51a757
^ permalink raw reply related
* Re: CodingGuidelines Perl amendment
From: Junio C Hamano @ 2013-02-06 18:14 UTC (permalink / raw)
To: demerphq; +Cc: Ted Zlatanov, git, Jeff King
In-Reply-To: <CANgJU+V5bhdpN_kWxQPEJgx24LXLtQJWRbnHwkSgm9zFwzm+fA@mail.gmail.com>
demerphq <demerphq@gmail.com> writes:
> As you mention below statement modifiers have their place. For instance
>
> next if $whatever;
>
> Is considered preferable to
>
> if ($whatever) {
> next;
> }
>
> Similarly
>
> open my $fh, ">", $filename
> or die "Failed to open '$filename': $!";
>
> Is considered preferable by most Perl programmers to:
>
> my $fh;
> if ( not open $fh, ">", $filename ) {
> die "Failed to open '$filename': $!";
> }
Yeah, and that is for the same reason. When you are trying to get a
birds-eye view of the codeflow, the former makes it clear that "we
do something, and then we open, and then we ...", without letting
the error handling (which also is rare case) distract us.
> "unless" often leads to maintenance errors as the expression gets more
> complicated over time,...
That might also be true, but my comment was not an endorsement for
(or suggestion against) use of unless. I was commenting on
statement modifiers, which some people tend to overuse (or abuse)
and make the resulting code harder to follow.
^ permalink raw reply
* Re: CodingGuidelines Perl amendment
From: Junio C Hamano @ 2013-02-06 18:16 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: git, Jeff King
In-Reply-To: <87vca5gvx6.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano <gitster@pobox.com> wrote:
>
> JCH> Is it ever (as opposed to "not always") possible to omit braces?
>
> Oh yes! Not that I recommend it, and I'm not even going to touch on
> Perl Golf :)
>
> JCH> It sounds as if we encourage the use of statement modifiers, which
> JCH> certainly is not what I want to see.
>
> Yup. I think I captured that in the patch, but please feel free to
> revise it after applying or throw it back to me.
I'd suggest to just drop that "try to write without braces" entirely.
> JCH> Incidentally, your sentence is a good example of where use of
> JCH> statement modifiers is appropriate: $youmust is rarely true.
>
> I was trying to be funny, honestly. But OK; reworded.
It wasn't a useful guidance, but it _was_ funny.
^ 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