* 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: [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 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: [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: 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
* [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: [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
* 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 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
* 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 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: 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: [RFC/PATCH 3/4] grep: allow to use textconv filters
From: Matthieu Moy @ 2013-02-06 15:12 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <da8c01b918c94c84ab61859b1b1453885bff5b06.1360162813.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Introduce an option "--textconv" which makes git grep use any configured
> textconv filters for grepping and output purposes. It is off by default.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> builtin/grep.c | 2 ++
> grep.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++--------
> grep.h | 1 +
Don't forget to update Documentation/git-grep.txt too.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Matthieu Moy @ 2013-02-06 15:10 UTC (permalink / raw)
To: Ted Zlatanov
Cc: Junio C Hamano, Jeff King, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <876225o5mj.fsf@lifelogs.com>
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,
Can you elaborate on this? The idea of the Perl code was to mimick a
call to the C API, keeping essentially the same names.
> and I wouldn't necessarily die() on error.
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()?
I don't know about the netrc credential helper, but I guess that's
another layer. The git-remote-mediawiki code is the code to call the
credential C API, that in turn may (or may not) call a credential
helper.
> Let me know if you'd like me to libify this... I'm happy to leave it to
> Matthieu or Michal, or anyone else interested.
I'd happily let you do the job, but I can help if needed. One thing to
be careful about: git-remote-mediawiki is currently a standalone script,
so it can be installed with a plain "cp git-remote-mediawiki $somewhere/".
One consequence of libification is that it adds a dependency on the
library (e.g. Git.pm). We should be carefull to keep it easy for the
user to install it (e.g. some kind of "make install", or update the doc).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* CodingGuidelines Perl amendment (was: [PATCH 1/3] Add contrib/credentials/netrc with GPG support)
From: Ted Zlatanov @ 2013-02-06 15:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7v7gmn1xqi.fsf@alter.siamese.dyndns.org>
On Mon, 04 Feb 2013 15:10:45 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
JCH> I thought that we tend to avoid Emacs/Vim formatting cruft left in
JCH> the file. Do we have any in existing file outside contrib/?
>>
>> No, but it's a nice way to express the settings so no one is guessing
>> what the project prefers. At least for me it's not an issue anymore,
>> since I understand your criteria better now, so let me know if you want
>> me to express it in the CodingGuidelines, in a dir-locals.el file, or
>> somewhere else.
JCH> Historically we treated this from CodingGuidelines a sufficient
JCH> clue:
JCH> As for more concrete guidelines, just imitate the existing code
JCH> (this is a good guideline, no matter which project you are
JCH> contributing to). It is always preferable to match the _local_
JCH> convention. New code added to git suite is expected to match
JCH> the overall style of existing code. Modifications to existing
JCH> code is expected to match the style the surrounding code already
JCH> uses (even if it doesn't match the overall style of existing code).
JCH> but over time people wanted more specific guidelines and added
JCH> language specific style guides there. We have sections that cover
JCH> C, shell and Python, and I do not think adding Perl would not hurt.
The following is how I have interpreted the Perl guidelines. I hope
it's OK to include Emacs-specific settings; they make it much easier to
reindent code to be acceptable.
I will submit as a patch if you think this is reasonable at all.
The org-mode markers around the code are just a suggestion.
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).
- Don't abuse statement modifiers (unless $youmust).
- 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:
#+begin_src lisp
((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))))
#+end_src
^ permalink raw reply
* [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
Make "grep" obey the "--textconv" option also for the object case, i.e.
when used with an argument "rev:path".
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
builtin/grep.c | 11 ++++++-----
object.c | 26 ++++++++++++++++++++------
object.h | 2 ++
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 915c8ef..0f3c4db 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -458,10 +458,10 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
}
static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
- struct object *obj, const char *name)
+ struct object *obj, const char *name, struct object_context *oc)
{
if (obj->type == OBJ_BLOB)
- return grep_sha1(opt, obj->sha1, name, 0, NULL);
+ return grep_sha1(opt, obj->sha1, name, 0, oc ? oc->path : NULL);
if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
struct tree_desc tree;
void *data;
@@ -503,7 +503,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
for (i = 0; i < nr; i++) {
struct object *real_obj;
real_obj = deref_tag(list->objects[i].item, NULL, 0);
- if (grep_object(opt, pathspec, real_obj, list->objects[i].name)) {
+ if (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].context)) {
hit = 1;
if (opt->status_only)
break;
@@ -820,14 +820,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
unsigned char sha1[20];
+ struct object_context oc;
/* Is it a rev? */
- if (!get_sha1(arg, sha1)) {
+ if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
struct object *object = parse_object(sha1);
if (!object)
die(_("bad object %s"), arg);
if (!seen_dashdash)
verify_non_filename(prefix, arg);
- add_object_array(object, arg, &list);
+ add_object_array_with_context(object, arg, &list, xmemdupz(&oc, sizeof(struct object_context)));
continue;
}
if (!strcmp(arg, "--")) {
diff --git a/object.c b/object.c
index 4af3451..1b796c7 100644
--- a/object.c
+++ b/object.c
@@ -245,12 +245,7 @@ int object_list_contains(struct object_list *list, struct object *obj)
return 0;
}
-void add_object_array(struct object *obj, const char *name, struct object_array *array)
-{
- add_object_array_with_mode(obj, name, array, S_IFINVALID);
-}
-
-void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode)
+static void add_object_array_with_mode_context(struct object *obj, const char *name, struct object_array *array, unsigned mode, struct object_context *context)
{
unsigned nr = array->nr;
unsigned alloc = array->alloc;
@@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
objects[nr].item = obj;
objects[nr].name = name;
objects[nr].mode = mode;
+ objects[nr].context = context;
array->nr = ++nr;
}
+void add_object_array(struct object *obj, const char *name, struct object_array *array)
+{
+ add_object_array_with_mode(obj, name, array, S_IFINVALID);
+}
+
+void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode)
+{
+ add_object_array_with_mode_context(obj, name, array, mode, NULL);
+}
+
+void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context)
+{
+ if (context)
+ add_object_array_with_mode_context(obj, name, array, context->mode, context);
+ else
+ add_object_array_with_mode_context(obj, name, array, S_IFINVALID, context);
+}
+
void object_array_remove_duplicates(struct object_array *array)
{
unsigned int ref, src, dst;
diff --git a/object.h b/object.h
index 6a97b6b..a11d719 100644
--- a/object.h
+++ b/object.h
@@ -13,6 +13,7 @@ struct object_array {
struct object *item;
const char *name;
unsigned mode;
+ struct object_context *context;
} *objects;
};
@@ -74,6 +75,7 @@ int object_list_contains(struct object_list *list, struct object *obj);
/* Object array handling .. */
void add_object_array(struct object *obj, const char *name, struct object_array *array);
void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode);
+void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context);
void object_array_remove_duplicates(struct object_array *);
void clear_object_flags(unsigned flags);
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* [RFC/PATCH 3/4] grep: allow to use textconv filters
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
From: Jeff King <peff@peff.net>
Recently and not so recently, we made sure that log/grep type operations
use textconv filters when a userfacing diff would do the same:
ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)
"git grep" currently does not use textconv filters at all, that is
neither for displaying the match and context nor for the actual grepping.
Introduce an option "--textconv" which makes git grep use any configured
textconv filters for grepping and output purposes. It is off by default.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
builtin/grep.c | 2 ++
grep.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++--------
grep.h | 1 +
3 files changed, 89 insertions(+), 14 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 8025964..915c8ef 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -659,6 +659,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
OPT_SET_INT('I', NULL, &opt.binary,
N_("don't match patterns in binary files"),
GREP_BINARY_NOMATCH),
+ OPT_BOOL(0, "textconv", &opt.allow_textconv,
+ N_("process binary files with textconv filters")),
{ OPTION_INTEGER, 0, "max-depth", &opt.max_depth, N_("depth"),
N_("descend at most <depth> levels"), PARSE_OPT_NONEG,
NULL, 1 },
diff --git a/grep.c b/grep.c
index 4bd1b8b..3880d64 100644
--- a/grep.c
+++ b/grep.c
@@ -2,6 +2,8 @@
#include "grep.h"
#include "userdiff.h"
#include "xdiff-interface.h"
+#include "diff.h"
+#include "diffcore.h"
static int grep_source_load(struct grep_source *gs);
static int grep_source_is_binary(struct grep_source *gs);
@@ -1321,6 +1323,58 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
fwrite(buf, size, 1, stdout);
}
+static int fill_textconv_grep(struct userdiff_driver *driver,
+ struct grep_source *gs)
+{
+ struct diff_filespec *df;
+ char *buf;
+ size_t size;
+
+ if (!driver || !driver->textconv)
+ return grep_source_load(gs);
+
+ /*
+ * The textconv interface is intimately tied to diff_filespecs, so we
+ * have to pretend to be one. If we could unify the grep_source
+ * and diff_filespec structs, this mess could just go away.
+ */
+ df = alloc_filespec(gs->path);
+ switch (gs->type) {
+ case GREP_SOURCE_SHA1:
+ fill_filespec(df, gs->identifier, 1, 0100644);
+ break;
+ case GREP_SOURCE_FILE:
+ fill_filespec(df, null_sha1, 0, 0100644);
+ break;
+ default:
+ die("BUG: attempt to textconv something without a path?");
+ }
+
+ /*
+ * fill_textconv is not remotely thread-safe; it may load objects
+ * behind the scenes, and it modifies the global diff tempfile
+ * structure.
+ */
+ grep_read_lock();
+ size = fill_textconv(driver, df, &buf);
+ grep_read_unlock();
+ free_filespec(df);
+
+ /*
+ * The normal fill_textconv usage by the diff machinery would just keep
+ * the textconv'd buf separate from the diff_filespec. But much of the
+ * grep code passes around a grep_source and assumes that its "buf"
+ * pointer is the beginning of the thing we are searching. So let's
+ * install our textconv'd version into the grep_source, taking care not
+ * to leak any existing buffer.
+ */
+ grep_source_clear_data(gs);
+ gs->buf = buf;
+ gs->size = size;
+
+ return 0;
+}
+
static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int collect_hits)
{
char *bol;
@@ -1331,6 +1385,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
unsigned count = 0;
int try_lookahead = 0;
int show_function = 0;
+ struct userdiff_driver *textconv = NULL;
enum grep_context ctx = GREP_CONTEXT_HEAD;
xdemitconf_t xecfg;
@@ -1352,19 +1407,36 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
}
opt->last_shown = 0;
- switch (opt->binary) {
- case GREP_BINARY_DEFAULT:
- if (grep_source_is_binary(gs))
- binary_match_only = 1;
- break;
- case GREP_BINARY_NOMATCH:
- if (grep_source_is_binary(gs))
- return 0; /* Assume unmatch */
- break;
- case GREP_BINARY_TEXT:
- break;
- default:
- die("bug: unknown binary handling mode");
+ if (opt->allow_textconv) {
+ grep_source_load_driver(gs);
+ /*
+ * We might set up the shared textconv cache data here, which
+ * is not thread-safe.
+ */
+ grep_attr_lock();
+ textconv = userdiff_get_textconv(gs->driver);
+ grep_attr_unlock();
+ }
+
+ /*
+ * We know the result of a textconv is text, so we only have to care
+ * about binary handling if we are not using it.
+ */
+ if (!textconv) {
+ switch (opt->binary) {
+ case GREP_BINARY_DEFAULT:
+ if (grep_source_is_binary(gs))
+ binary_match_only = 1;
+ break;
+ case GREP_BINARY_NOMATCH:
+ if (grep_source_is_binary(gs))
+ return 0; /* Assume unmatch */
+ break;
+ case GREP_BINARY_TEXT:
+ break;
+ default:
+ die("bug: unknown binary handling mode");
+ }
}
memset(&xecfg, 0, sizeof(xecfg));
@@ -1372,7 +1444,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
try_lookahead = should_lookahead(opt);
- if (grep_source_load(gs) < 0)
+ if (fill_textconv_grep(textconv, gs) < 0)
return 0;
bol = gs->buf;
diff --git a/grep.h b/grep.h
index 8fc854f..94a7ac2 100644
--- a/grep.h
+++ b/grep.h
@@ -106,6 +106,7 @@ struct grep_opt {
#define GREP_BINARY_NOMATCH 1
#define GREP_BINARY_TEXT 2
int binary;
+ int allow_textconv;
int extended;
int use_reflog_filter;
int pcre;
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* [RFC/PATCH 2/4] cat-file: do not die on --textconv without textconv filters
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
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".
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";
case 0:
if (type_from_string(exp_type) == OBJ_BLOB) {
diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
index 78a0085..83c6636 100755
--- a/t/t8007-cat-file-textconv.sh
+++ b/t/t8007-cat-file-textconv.sh
@@ -22,11 +22,11 @@ test_expect_success 'setup ' '
'
cat >expected <<EOF
-fatal: git cat-file --textconv: unable to run textconv on :one.bin
+bin: test version 2
EOF
test_expect_success 'no filter specified' '
- git cat-file --textconv :one.bin 2>result
+ git cat-file --textconv :one.bin >result &&
test_cmp expected result
'
@@ -36,10 +36,6 @@ test_expect_success 'setup textconv filters' '
git config diff.test.cachetextconv false
'
-cat >expected <<EOF
-bin: test version 2
-EOF
-
test_expect_success 'cat-file without --textconv' '
git cat-file blob :one.bin >result &&
test_cmp expected result
@@ -73,25 +69,19 @@ test_expect_success 'cat-file --textconv on previous commit' '
'
test_expect_success SYMLINKS 'cat-file without --textconv (symlink)' '
+ printf "%s" "one.bin" >expected &&
git cat-file blob :symlink.bin >result &&
- printf "%s" "one.bin" >expected
test_cmp expected result
'
test_expect_success SYMLINKS 'cat-file --textconv on index (symlink)' '
- ! git cat-file --textconv :symlink.bin 2>result &&
- cat >expected <<\EOF &&
-fatal: git cat-file --textconv: unable to run textconv on :symlink.bin
-EOF
+ git cat-file --textconv :symlink.bin >result &&
test_cmp expected result
'
test_expect_success SYMLINKS 'cat-file --textconv on HEAD (symlink)' '
- ! git cat-file --textconv HEAD:symlink.bin 2>result &&
- cat >expected <<EOF &&
-fatal: git cat-file --textconv: unable to run textconv on HEAD:symlink.bin
-EOF
+ git cat-file --textconv HEAD:symlink.bin >result &&
test_cmp expected result
'
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
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.
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;
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* [RFC/PATCH 0/4] textconv for show and grep
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <20130205201106.GA29248@sigill.intra.peff.net>
This min series aims at completing the textconv support of user facing
commands. It is RFC for lack of documentation and tests, to check
whether we really want to go in that direction (I do).
1/4 covers the missing textconv support in the "blob case" of "git
show", which should be (and then is) analogous to "cat-file" and "diff".
2/4 remedies an oddity of "git cat-file --textconv", which errored out
when there is no textconv filter rather than giving an unfiltered blob
(like every other textconv aware command).
3/4 implements "--textconv" for "git grep" sans the blob case; the code
is all Jeff's.
4/4 adds blob support to 3/4 (the "rev:path" case).
3 and 4 can be squashed, of course.
Now I'm quite a happy differ/shower/grepper with my latin1 and OO files ;)
Jeff King (1):
grep: allow to use textconv filters
Michael J Gruber (3):
show: obey --textconv for blobs
cat-file: do not die on --textconv without textconv filters
grep: obey --textconv for the case rev:path
builtin/cat-file.c | 9 ++--
builtin/grep.c | 13 +++---
builtin/log.c | 24 +++++++++--
grep.c | 100 +++++++++++++++++++++++++++++++++++++------
grep.h | 1 +
object.c | 26 ++++++++---
object.h | 2 +
t/t8007-cat-file-textconv.sh | 20 +++------
8 files changed, 148 insertions(+), 47 deletions(-)
--
1.8.1.2.752.g32d147e
^ permalink raw reply
* Re: Bug in "git log --graph -p -m" (version 1.7.7.6)
From: Dale R. Worley @ 2013-02-06 15:03 UTC (permalink / raw)
To: Matthieu Moy; +Cc: gitster, git
In-Reply-To: <vpqmwvia2n7.fsf@grenoble-inp.fr>
> 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 "| * ":
> | |
> | * commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 33e70e70c0173d634826b998bdc304f93c0966b8)
> | | Merge: 2c1e6a3 33e70e7
> | | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
> | | Date: Tue Feb 5 22:05:33 2013 +0100
Dale
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-02-06 14:53 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, Jeff King, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <vpqa9rhaml6.fsf@grenoble-inp.fr>
On Wed, 06 Feb 2013 09:11:17 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
MM> Junio C Hamano <gitster@pobox.com> writes:
>> I see a lot of rerolls on the credential helper front, but is there
>> anybody working on hooking send-email to the credential framework?
MM> Not answering the question, but git-remote-mediawiki supports the
MM> credential framework. It is written in perl, and the credential support
MM> is rather cleanly written and doesn't have dependencies on the wiki
MM> part, 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, and I wouldn't necessarily die() on error. Maybe this can
be merged with the netrc credential helper's
read_credential_data_from_stdin() and print_credential_data()?
Let me know if you'd like me to libify this... I'm happy to leave it to
Matthieu or Michal, or anyone else interested.
Ted
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-02-06 14:47 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Junio C Hamano, Jeff King, git, Krzysztof Mazur
In-Reply-To: <xa1tpq0dpo89.fsf@mina86.com>
On Wed, 06 Feb 2013 14:26:46 +0100 Michal Nazarewicz <mina86@mina86.com> wrote:
MN> On Wed, Feb 06 2013, Junio C Hamano <gitster@pobox.com> wrote:
>> I see a lot of rerolls on the credential helper front, but is there
>> anybody working on hooking send-email to the credential framework?
MN> I assumed someone had, but if not I can take a stab at it. I'm not sure
MN> however how should I map server, server-port, and user to credential
MN> key-value pairs. I'm leaning towards
MN> protocol=smtp
MN> host=<smtp-server>:<smtp-port>
MN> user=<user>
MN> and than netrc/authinfo helper splitting host to host name and port
MN> number, unless port is not in host in which case protocol is assumed as
MN> port.
That would work (with my PATCHv6 of the netrc credential helper) as
follows:
1) just host
host=H
maps to
machine H login Y password Z
2) host + protocol smtp
host=H
protocol=smtp
maps to any of:
machine H port smtp login Y password Z
machine H protocol smtp login Y password Z
3) host:port + protocol smtp
host=H:25
protocol=smtp
maps to any of:
machine H port 25 protocol smtp login Y password Z
machine H:25 port smtp login Y password Z
machine H:25 protocol smtp login Y password Z
That's my understanding of what we discussed with Peff and Junio about
token mapping. Note we don't split the input host, but instead say "if
token 'port' is numeric, append it to the host token" on the netrc side.
Does that sound reasonable? If yes, I can add it to the testing
Makefile for the netrc credential helper, to make sure it's clearly
stated and tested.
Ted
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Michal Nazarewicz @ 2013-02-06 13:26 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: git, Krzysztof Mazur
In-Reply-To: <7v6226pdb7.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 874 bytes --]
On Wed, Feb 06 2013, Junio C Hamano <gitster@pobox.com> wrote:
> I see a lot of rerolls on the credential helper front, but is there
> anybody working on hooking send-email to the credential framework?
I assumed someone had, but if not I can take a stab at it. I'm not sure
however how should I map server, server-port, and user to credential
key-value pairs. I'm leaning towards
protocol=smtp
host=<smtp-server>:<smtp-port>
user=<user>
and than netrc/authinfo helper splitting host to host name and port
number, unless port is not in host in which case protocol is assumed as
port.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply
* Why is ident_is_sufficient different on Windows?
From: Max Horn @ 2013-02-06 13:06 UTC (permalink / raw)
To: git
Hi there,
while trying to understand which parts of the author & committer identity are mandatory (name, email, or both), I ended up in ident.c, looking at ident_is_sufficient(), and to my surprise discovered that this seems to differ between Windows (were both are mandatory) and everyone else:
static int ident_is_sufficient(int user_ident_explicitly_given)
{
#ifndef WINDOWS
return (user_ident_explicitly_given & IDENT_MAIL_GIVEN);
#else
return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
#endif
}
According to git blame, this was introduced here:
commit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e
Author: Junio C Hamano <gitster@pobox.com>
Date: Sun Jan 17 13:54:28 2010 -0800
user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere
The commit message sounds as if this was only a refactoring, but the patch to me look as if it changes behaviour, too. Of course this could very well be false, say due to code elsewhere that already caused Windows to behave differently; I wouldn't know.
Still, I wonder: Why does this difference exist?
Cheers,
Max
^ 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