* Re: [PATCH] index-pack: correctly initialize appended objects
From: Nicolas Pitre @ 2008-07-25 12:24 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: Johannes Schindelin, Junio C Hamano, spearce, git
In-Reply-To: <20080725120102.GB32487@atjola.homenet>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1618 bytes --]
On Fri, 25 Jul 2008, Björn Steinbrink wrote:
> On 2008.07.25 07:54:49 -0400, Nicolas Pitre wrote:
> > On Fri, 25 Jul 2008, Johannes Schindelin wrote:
> >
> > > Hi,
> > >
> > > On Thu, 24 Jul 2008, Junio C Hamano wrote:
> > >
> > > > The function does not seem to use type (which the patch is also setting)
> > > > nor real_type (which the patch does not set).
> > > >
> > > > However, the code checks objects[nth].real_type all over the place in
> > > > the code. Doesn't the lack of real_type assignment in
> > > > append_obj_to_pack() affect them in any way?
> > >
> > > >From staring at the code, I thought that real_type was set in
> > > resolve_delta(), but I may be wrong.
> > >
> > > The safer thing would be to set it, but I am not quite sure if we can use
> > > "type" directly, or if type can be "delta" for an object that is used to
> > > complete the pack, and therefore stored as a non-delta.
> >
> > Objects to complete the pack are always non delta, so the type and
> > real_type should be the same. However that shouldn't matter since at
> > that point the object array is not walked anymore, at least not for
> > appended objects, and therefore initializing the type at that point is
> > redundant.
>
> Is that still true when the object has been pruned due to memory
> constraints set by deltaBaseCacheLimit? AFAICT when reloading the data
> for the object, we end up in get_base_data, which at least checks
> obj->type.
yeah, true. I don't really have this new code path in my head yet.
In any case, appended objects should have type = real_type = non delta
type.
Nicolas
^ permalink raw reply
* Re: [PATCH] Respect crlf attribute even if core.autocrlf has not been set
From: Eyvind Bernhardsen @ 2008-07-25 12:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Avery Pennarun, Joshua Jensen, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0807250159420.4140@eeepc-johanness>
On 25. juli. 2008, at 02.01, Johannes Schindelin wrote:
>> Um, what? This discussion is about figuring out when Git should mess
>> with the line endings a user is trying to commit.
>
> This discusion is about when Git should mess with _content_ at all.
>
> It is not limited to line endings.
Fair enough. Did you read the rest of my email to see when I think
Git should mess with content? I've thought about it, and being able
to do stuff like this in .gitattributes would work for me:
* eol=auto
*.bat eol=crlf
Makefile eol=lf
bin/magic-binary eol=none
I.e. "detect line endings and do CRLF->LF conversion on all files
except *.bat (*->CRLF), Makefile (*->LF) and bin/magic-binary (do
nothing)".
The closest you can get today is by setting core.autocrlf to "input"
and having this in .gitattributes:
*.bat -crlf
bin/magic-binary -crlf
...but "core.autocrlf" is not versioned and applies to _all_
repositories, and anyone who doesn't have the correct setting can mess
the repository up. There's also no way of saying "this file should
have LF line endings, even with autocrlf=true".
One problem is that the autocrlf setting mixes "I want LF only in my
repositories" and "I like to have CRLFs in my working directory" into
one config variable. Instead, I'd like to have a config setting that
specifies which line ending form I prefer: "when a text file is marked
eol=auto, convert LFs to CRLFs on checkout".
Does this make sense to anyone but me?
> Ciao,
> Dscho "who personally could not care less about CR if it was not for
> M$'
> stupidity"
Well, we agree on that.
--
Eyvind Bernhardsen
^ permalink raw reply
* [PATCH] Avoid warning when From: is encoded
From: Peter Valdemar Mørch @ 2008-07-25 13:06 UTC (permalink / raw)
To: git
From: Peter Valdemar Mørch <peter@morch.com>
In commit 0706bd19ef9b41e7519df2c73796ef93484272fd $1 is used from a regexp
without using () to set up $1. Later, when that value was used, it caused a
warning about a variable being undefined.
Signed-off-by: Peter Valdemar Mørch <peter@morch.com>
---
The commit introduces $body_encoding and: $body_encoding = $1; which is undef.
That commit then later uses $body_encoding only here:
+ if ($has_content_type) {
+ if ($body_encoding eq $author_encoding) {
+ # ok, we already have the right encoding
+ }
+ else {
+ # uh oh, we should re-encode
+ }
+ }
(I removed some whitespace for readability)
.. and it was the eq that gave the warning, because $body_encoding was
undefined. Perhaps a better fix is to remove $body_encoding and regexp
altogether since it isn't really used. Let me know if you think so.
This is where my non-commit message goes, yeah? I'm hand editing the output of
'git format-patch'...
Junio C. Hamano commented on a previous post that I shouldn't send patches as
attachments so now I'm trying git-send-email. Are there any form problems with
this patch?
git-send-email.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 2e4a44a..d2fd899 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -882,7 +882,7 @@ foreach my $t (@files) {
}
elsif (/^Content-type:/i) {
$has_content_type = 1;
- if (/charset="?[^ "]+/) {
+ if (/charset="?([^ "]+)/) {
$body_encoding = $1;
}
push @xh, $_;
--
1.6.0.rc0.46.g07955.dirty
^ permalink raw reply related
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Johannes Schindelin @ 2008-07-25 13:15 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: Junio C Hamano, Nicolas Pitre, spearce, git
In-Reply-To: <20080725115519.GA32487@atjola.homenet>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1011 bytes --]
Hi,
On Fri, 25 Jul 2008, Björn Steinbrink wrote:
> On 2008.07.24 22:21:14 -0700, Junio C Hamano wrote:
> > Reading get_data_from_pack(), it does rely on hdr_size, idx.offset and
> > idx.offset of the next entry to be set correctly. The function does
> > not seem to use type (which the patch is also setting) nor real_type
> > (which the patch does not set).
>
> type is used in get_base_data().
>
> > However, the code checks objects[nth].real_type all over the place in
> > the code. Doesn't the lack of real_type assignment in
> > append_obj_to_pack() affect them in any way?
>
> I had thought that resolve_delta() would set that, but it seems that we
> never call that function like that. Hm...
So, let's add the comment as Nico suggested, and set real_type, too? (And
it would be smashing if you could verify that the type is indeed correctly
set to non-delta...)
I think that setting real_type is necessary to have less surprises when
the code is extended in the future.
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH] bash completion: Add long options for 'git describe'
From: SZEDER Gábor @ 2008-07-25 13:15 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, gitster, Shawn O. Pearce
In-Reply-To: <1216980170-14136-1-git-send-email-trast@student.ethz.ch>
Hi,
On Fri, Jul 25, 2008 at 12:02:50PM +0200, Thomas Rast wrote:
> ---
Signed off by?
> _git_describe ()
> {
> + __git_has_doubledash && return
This line is superfluous, because 'git describe' does not have any path
arguments.
Gábor
^ permalink raw reply
* Re: [PATCH] Avoid warning when From: is encoded
From: Abhijit Menon-Sen @ 2008-07-25 13:16 UTC (permalink / raw)
To: Peter Valdemar Mørch; +Cc: git
In-Reply-To: <1216991208-18782-1-git-send-email-4ux6as402@sneakemail.com>
At 2008-07-25 15:06:48 +0200, 4ux6as402@sneakemail.com wrote:
>
> This is where my non-commit message goes, yeah?
Yes.
> Are there any form problems with this patch?
Looks fine to me (and also to git am).
The patch itself also looks good to me (but I'm not sure if that means I
should add an Acked-by: line to this message).
-- ams
^ permalink raw reply
* Re: statistics
From: Johannes Schindelin @ 2008-07-25 13:23 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Rene Herman, git
In-Reply-To: <4889B912.9040006@viscovery.net>
Hi,
On Fri, 25 Jul 2008, Johannes Sixt wrote:
> comm -13 <(git diff --name-only your-rev-here) <(git ls-files)
Thanks. I learnt two new things from that: comm (funnily, I did not know
that tool), and <() (looks almost like a puffin, doesn't it?).
But should it not be -12?
Ciao,
Dscho
^ permalink raw reply
* Re: statistics
From: Johannes Sixt @ 2008-07-25 13:34 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Rene Herman, git
In-Reply-To: <alpine.DEB.1.00.0807251519290.11976@eeepc-johanness>
Johannes Schindelin schrieb:
> On Fri, 25 Jul 2008, Johannes Sixt wrote:
>
>> comm -13 <(git diff --name-only your-rev-here) <(git ls-files)
>
> But should it not be -12?
I don't think so:
-1 .. suppress lines unique to 1st arg, i.e. removed files
-2 .. suppress lines unique to 2nd arg, i.e. unmodified files
-3 .. suppress lines in both, i.e. modified and added files
We want to keep lines that -2 would remove, that leaves -1 and -3.
-- Hannes
^ permalink raw reply
* Re: statistics
From: Johannes Schindelin @ 2008-07-25 13:41 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Rene Herman, git
In-Reply-To: <4889D66E.9090802@viscovery.net>
Hi,
On Fri, 25 Jul 2008, Johannes Sixt wrote:
> Johannes Schindelin schrieb:
> > On Fri, 25 Jul 2008, Johannes Sixt wrote:
> >
> >> comm -13 <(git diff --name-only your-rev-here) <(git ls-files)
> >
> > But should it not be -12?
>
> I don't think so:
>
> -1 .. suppress lines unique to 1st arg, i.e. removed files
> -2 .. suppress lines unique to 2nd arg, i.e. unmodified files
> -3 .. suppress lines in both, i.e. modified and added files
>
> We want to keep lines that -2 would remove, that leaves -1 and -3.
Oh, but of course!
Thanks again,
Dscho
^ permalink raw reply
* Re: [PATCH] Respect crlf attribute even if core.autocrlf has not been set
From: Dmitry Potapov @ 2008-07-25 14:01 UTC (permalink / raw)
To: Eyvind Bernhardsen
Cc: Johannes Schindelin, Avery Pennarun, Joshua Jensen,
Junio C Hamano, git
In-Reply-To: <E0666371-5C5E-4AA9-B67A-16C42477865B@orakel.ntnu.no>
On Fri, Jul 25, 2008 at 02:30:16PM +0200, Eyvind Bernhardsen wrote:
>
> Fair enough. Did you read the rest of my email to see when I think
> Git should mess with content? I've thought about it, and being able
> to do stuff like this in .gitattributes would work for me:
>
> * eol=auto
> *.bat eol=crlf
> Makefile eol=lf
> bin/magic-binary eol=none
>
> I.e. "detect line endings and do CRLF->LF conversion on all files
> except *.bat (*->CRLF), Makefile (*->LF) and bin/magic-binary (do
> nothing)".
I suppose "* eol=auto" means to convert CRLF->LF on checkin and
LF->native on checkout?
Also, perhaps, it should be also possible to explicitly specify:
*.txt eol=native
which is the same as 'auto' but without guessing whether it is text
or not.
> ...but "core.autocrlf" is not versioned and applies to _all_
> repositories, and anyone who doesn't have the correct setting can mess
> the repository up.
I think the real issue here is not as much about being or not being
versioned, but about forcing and not forcing anything on users.
If we had core.autocrlf=input as default then clueless users will not
checkin files with the incorrect ending. But there is an objection to
that -- you penalize those who always have good endings. And even the
fact that is merely default value that you can easily change to false
does not convince everyone.
The same can be said about your
* eol=auto
It forces conversion on everyone, even on those who do not need it.
Of course, you can say those projects that do not have the problem with
clueless users putting text files with incorrect end-of-lines will not
have lines like that in their .gitattribute. Yet, if I participate in
that project, why do I have to pay the price for this conversion just
because someone stupid can mess up line-endings?
> There's also no way of saying "this file should
> have LF line endings, even with autocrlf=true".
Actually, there is
*.sh crlf=input
i.e. I want my shell files to have LF even I normally use CRLF for
all other files (on Windows).
>
> One problem is that the autocrlf setting mixes "I want LF only in my
> repositories" and "I like to have CRLFs in my working directory" into
> one config variable. Instead, I'd like to have a config setting that
> specifies which line ending form I prefer: "when a text file is marked
> eol=auto, convert LFs to CRLFs on checkout".
Following your style above, I believe it should be defined as
native-eol=crlf
but there are people who do not want to pay any price for conversion.
Currently, "core.autocrlf=false" means to do nothing about end-of-lines,
and even to ignore setting in .gitattributes. Should it be possible to
disable *any* conversion on checkin and checkout? Should this be that
value be the default, which most users use?
Dmitry
--
If you write a program that any idiot can use, only idiots will use it.
^ permalink raw reply
* Re: [EGIT] Supported Eclipse version
From: Marek Zawirski @ 2008-07-25 14:12 UTC (permalink / raw)
To: Jean-François Veillette; +Cc: git, Robin Rosenberg, Shawn O. Pearce
In-Reply-To: <89D1384A-403C-4E6E-816C-204AE0AAC30C@yahoo.ca>
Jean-François Veillette wrote:
>> Maybe some users (or developers) from mailing list can tell us about
>> their opinion?
>
> I think keeping comptability for one version behind the 'current'
> (still compatible 3.3 while 3.4 is the current) is a reasonable goal.
> 3.2 is relatively far behind, tools vendor had time to get up to date
> by now.
> If a user is using a third party version of eclipse based on 3.2, then
> I guess he should not expect egit to work with it (he should just pray
> and hope for the best).
>
> For myself, I use 3.3.x, I have not yet moved to 3.4 since my main
> plugin (WOLips) is not ready yet for 3.4.
>
> - jfv
I would also vote for this change. Especially if there are not many
users screaming that they need egit in Eclipse 3.2.
BTW, I'm using 3.4 version for 2 days and it looks really nice. What's
important, it seems to not be that buggy as early 3.3 releases were.
Robin, Shawn, would you accept next patches using 3.3 API?
Marek
^ permalink raw reply
* Re: [PATCH] editor.c: Libify launch_editor()
From: Stephan Beyer @ 2008-07-25 14:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0807251226500.11976@eeepc-johanness>
Hi,
Johannes Schindelin wrote:
> On Fri, 25 Jul 2008, Junio C Hamano wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > > On Fri, 18 Jul 2008, Stephan Beyer wrote:
> > >
> > >> diff --git a/editor.c b/editor.c
> > >> index 483b62d..5d7f5f9 100644
> > >> --- a/editor.c
> > >> +++ b/editor.c
> > >> @@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
> > > ...
> > > Why not "return error()"?
> > >
> > > Rest looks obviously correct to me!
> >
> > This is a patch to an existing file editor.c???
>
> Yes, Stephan sent the two patches unrelatedly,
Yes, because when I've sent 1/2, I didn't knew that 2/2 will exist.
So I've just sent the second patch "into this thread" and hoped it would
be clear how it is meant. :)
So, to summarize:
[PATCH 1/2] http://thread.gmane.org/gmane.comp.version-control.git/88940/focus=88940
[PATCH 2/2] http://thread.gmane.org/gmane.comp.version-control.git/88940/focus=89035
They still apply cleanly to current master.
> even if they should have been marked "1/2" and "2/2".
> I hope he does so now.
This means I should send them again?
Regards.
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: [PATCH] editor.c: Libify launch_editor()
From: Johannes Schindelin @ 2008-07-25 14:36 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Junio C Hamano, git
In-Reply-To: <20080725141517.GB18198@leksak.fem-net>
Hi,
On Fri, 25 Jul 2008, Stephan Beyer wrote:
> Johannes Schindelin wrote:
>
> > even if they should have been marked "1/2" and "2/2". I hope he does
> > so now.
>
> This means I should send them again?
If you want to be nice to the maintainer, yes. If not, no.
Ciao,
Dscho
^ permalink raw reply
* Re: problem using jgit
From: Marek Zawirski @ 2008-07-25 14:51 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Stephen Bannasch, git, Robin Rosenberg
In-Reply-To: <20080722165831.GA11173@spearce.org>
Shawn O. Pearce wrote:
> Marek Zawirski <marek.zawirski@gmail.com> wrote:
>
>> Marek Zawirski wrote:
>>
>>> Stephen Bannasch wrote:
>>>
>>>> I've setup a simple test class that integrates jgit to clone a git
>>>> repository. However I'm getting a NullPointerError when
>>>> RevWalk.parseAny ends up producing a null object id.
>>>>
> ...
>
>> It's caused by 14a630c3: Cached modification times for symbolic refs too
>> Changes introduced by this patch made Repository#getAllRefs() including
>> Ref objects with null ObjectId in case of unresolvable (invalid?) HEAD
>> symbolic ref, and null Ref for HEAD when it doesn't exist. Previous
>> behavior was just not including such refs in result.
>>
>
> My intention here was that if a ref cannot be resolved, it should
> not be reported. So Ref.getObjectId should never return null, and
> it should also never return an ObjectId for which the object does
> not exist in the Repository's object database(s). (Though that can
> happen in the face of repository corruption, but lets not go there
> just yet).
>
> So IMHO the RefDatabase code is _wrong_ for returning HEAD with a
> null objectId.
>
> Now this case can happen if HEAD points at a stillborn branch. This
> is easily reproduced in any repository, e.g. just do:
>
> git symbolic-ref HEAD refs/heads/`date`
>
> You'll wind up on a branch which doesn't exist. In this case HEAD
> shouldn't be reported back from RefDatabase, it doesn't exist, as
> branch `date` does not exist either.
>
>
Beside of my temporary fix for that that filters null Ref and Ref with
null objectId, I think that 2 more issues may need to be resolved:
1) readRefBasic() method is used for reading arbitrary refs, potentially
not only those from well-known prefixes as readRefs() does. Is calling
setModified() appropriate for those other refs?
2) Am I wrong that setModified() is not called in all cases? Consider
empty ref file and just...
if (line == null || line.length() == 0)
return new Ref(Ref.Storage.LOOSE, name, null);
^ permalink raw reply
* Re: [RFC PATCH 00/12] Sparse checkout
From: Avery Pennarun @ 2008-07-25 14:53 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Jakub Narebski, git, Johannes Schindelin
In-Reply-To: <fcaeb9bf0807241621y60b0341ej8f9f3b591ef12baf@mail.gmail.com>
On 7/24/08, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> I wrote that with svn repos in mind. If those repos are to be
> partially checked out, .svnignore would be in subdirectories rather
> than at toplevel. Anyway that may not be true.
As far as I know, this is a misstatement of how svn works. It uses
svn:ignore properties, not .svnignore files, and they don't apply
recursively, so this potential inconsistency wouldn't happen with svn.
Of course, in the name of this consistency, svn gave up recursive
ignores.
So anyway, back on topic, the definition of a "well-organized
repository" is different wth git than with svn for this reason. I
would certainly expect most well-organized git repositories to have a
toplevel .gitignore for the most common stuff.
Have fun,
Avery
^ permalink raw reply
* Re: [PATCH 6/9] builtin-init-db.c: use parse_options()
From: Olivier Marin @ 2008-07-25 15:20 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Michele Ballabio, git, gitster
In-Reply-To: <alpine.DEB.1.00.0807241801030.8986@racer>
Johannes Schindelin a écrit :
>>
>>> + OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"),
>> s/set up/setup/
>
> No. "setup" is a noun.
Right, sorry.
> We rely on shared_repository == 0 for non-shared repositories _almost
> everywhere_.
I think we rely on the fact that PERM_UMASK == 0 and not on the value
of shared_repository. Not the same thing.
That said, perhaps you are right: it is harmless.
>>> + OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
>> OPT__QUIET(&quiet),
>>
>> if (quiet)
>> flags |= INIT_DB_QUIET;
>>
>> to use the same quiet option everywhere?
>
> Why? Doesn't make it more readable, I think. I'd rather have 3 lines
> less.
Hum.
Olivier.
^ permalink raw reply
* Re: [PATCH 1/9] builtin-verify-tag.c: use parse_options()
From: Olivier Marin @ 2008-07-25 15:20 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Michele Ballabio, git, gitster
In-Reply-To: <alpine.DEB.1.00.0807241807550.8986@racer>
Johannes Schindelin a écrit :
>
> That would be a bugfix. As such, it belongs into a different commit.
I thought, for that kind of trivial bug that probably never hit anyone,
a line in the commit message was enough.
> Care to provide a patch?
OK, will do.
Olivier.
^ permalink raw reply
* Re: [PATCH 6/9] builtin-init-db.c: use parse_options()
From: Olivier Marin @ 2008-07-25 15:20 UTC (permalink / raw)
To: Michele Ballabio; +Cc: git, gitster
In-Reply-To: <200807242207.02195.barra_cuda@katamail.com>
Michele Ballabio a écrit :
>
>>> + PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
>> Are you sure the default value is really used here?
>
> Yes. Perhaps I don't understand your question. Can you explain what you mean?
If I read the code correctly in parse-options.c, with OPTION_CALLBACK, the
default value is not "automatically" used. You can use it in your callback
if you want, but because you don't, I think it's never used.
> Would you like this better, with PARSE_OPT_NONEG?
No, I'm fine with the negated option.
> Or do you prefer changing the callback like this:
>
> +static int parse_opt_shared_cb(const struct option *opt, const char *arg,
> + int unset)
> +{
> + *(int *)(opt->value) = unset ? PERM_UMASK : git_config_perm("arg", arg);
> + return 0;
> +}
I think it's better but what I suggested is more something like:
static int parse_opt_shared_cb(const struct option *opt, const char *arg,
int unset)
{
*(int *)(opt->value) = unset ? -1 : git_config_perm("arg", arg);
return 0;
}
int shared = -1;
{ OPTION_CALLBACK, 0, "shared", &shared,
"permissions", "setup as shared repository",
PARSE_OPT_OPTARG, parse_perm_callback },
if (shared >= 0)
shared_repository = shared;
This way we do not change shared_repository during parsing, so we do not
loose the initial value.
But it seems nobody care about this kind of details, so perhaps, you can
just ignore this suggestion.
Olivier.
^ permalink raw reply
* [PATCH] git-svn: teach dcommit about svn auto-props
From: Brad King @ 2008-07-25 15:32 UTC (permalink / raw)
To: git; +Cc: Eric Wong
In-Reply-To: <20080725060037.GB14756@untitled>
[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]
Subversion repositories often require files to have properties such as
svn:mime-type and svn:eol-style set when they are added. Users
typically set these properties automatically using the SVN auto-props
feature with 'svn add'. This commit teaches dcommit to look at the user
SVN configuration and apply matching auto-props entries for files added
by a diff as it is applied to the SVN remote.
Signed-off-by: Brad King <brad.king@kitware.com>
---
Eric Wong wrote:
> I like this patch.
Thanks.
> Can we get an automated test of this functionality?
This patch adds a test. I also fixed the property name/value parsing
to remove leading and trailing whitespace.
> We can (and probably should) set $HOME for the test and ignore the
> existing ~/.subversion/config of the user.
I used the --config-dir option.
> Also, some minor nitpicks on whitespace/formatting inline below.
Addressed. I missed the wrong indentation before because my second patch
removed it.
> I haven't had the chance to look at this. Can anybody else shed more
> light on that bug? It's really strange that the tests won't run because
> of it. Are you unable to run some git-svn tests or all of them?
Just that one fails. All others (including the one in the patch below) pass.
Thanks for reviewing,
-Brad
git-svn.perl | 52 ++++++++++++++++++++
t/t9124-git-svn-dcommit-auto-props.sh | 84 +++++++++++++++++++++++++++++++++
2 files changed, 136 insertions(+), 0 deletions(-)
create mode 100755 t/t9124-git-svn-dcommit-auto-props.sh
[-- Attachment #2: a13a0b82dc4d8a4d45a94bdc58a60b605debe17a.diff --]
[-- Type: text/x-patch, Size: 4794 bytes --]
diff --git a/git-svn.perl b/git-svn.perl
index 2e0e552..0a8e907 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3340,6 +3340,7 @@ sub new {
$self->{rm} = { };
$self->{path_prefix} = length $self->{svn_path} ?
"$self->{svn_path}/" : '';
+ $self->{config} = $opts->{config};
return $self;
}
@@ -3528,6 +3529,56 @@ sub ensure_path {
return $bat->{$c};
}
+# Subroutine to convert a globbing pattern to a regular expression.
+# From perl cookbook.
+sub glob2pat {
+ my $globstr = shift;
+ my %patmap = ('*' => '.*', '?' => '.', '[' => '[', ']' => ']');
+ $globstr =~ s{(.)} { $patmap{$1} || "\Q$1" }ge;
+ return '^' . $globstr . '$';
+}
+
+sub check_autoprop {
+ my ($self, $pattern, $properties, $file, $fbat) = @_;
+ # Convert the globbing pattern to a regular expression.
+ my $regex = glob2pat($pattern);
+ # Check if the pattern matches the file name.
+ if($file =~ m/($regex)/) {
+ # Parse the list of properties to set.
+ my @props = split(/;/, $properties);
+ foreach my $prop (@props) {
+ # Parse 'name=value' syntax and set the property.
+ if ($prop =~ /([^=]+)=(.*)/) {
+ my ($n,$v) = ($1,$2);
+ $n =~ s/^\s+//; $n =~ s/\s+$//;
+ $v =~ s/^\s+//; $v =~ s/\s+$//;
+ $self->change_file_prop($fbat, $n, $v);
+ }
+ }
+ }
+}
+
+sub apply_autoprops {
+ my ($self, $file, $fbat) = @_;
+ my $conf_t = ${$self->{config}}{'config'};
+ no warnings 'once';
+ # Check [miscellany]/enable-auto-props in svn configuration.
+ if (SVN::_Core::svn_config_get_bool(
+ $conf_t,
+ $SVN::_Core::SVN_CONFIG_SECTION_MISCELLANY,
+ $SVN::_Core::SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS,
+ 0)) {
+ # Auto-props are enabled. Enumerate them to look for matches.
+ my $callback = sub {
+ $self->check_autoprop($_[0], $_[1], $file, $fbat);
+ };
+ SVN::_Core::svn_config_enumerate(
+ $conf_t,
+ $SVN::_Core::SVN_CONFIG_SECTION_AUTO_PROPS,
+ $callback);
+ }
+}
+
sub A {
my ($self, $m) = @_;
my ($dir, $file) = split_path($m->{file_b});
@@ -3535,6 +3586,7 @@ sub A {
my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
undef, -1);
print "\tA\t$m->{file_b}\n" unless $::_q;
+ $self->apply_autoprops($file, $fbat);
$self->chg_file($fbat, $m);
$self->close_file($fbat,undef,$self->{pool});
}
diff --git a/t/t9124-git-svn-dcommit-auto-props.sh b/t/t9124-git-svn-dcommit-auto-props.sh
new file mode 100755
index 0000000..beefbcc
--- /dev/null
+++ b/t/t9124-git-svn-dcommit-auto-props.sh
@@ -0,0 +1,84 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Brad King
+
+test_description='git-svn dcommit honors auto-props'
+
+. ./lib-git-svn.sh
+
+generate_auto_props() {
+cat << EOF
+[miscellany]
+enable-auto-props=$1
+[auto-props]
+*.sh = svn:mime-type=application/x-shellscript; svn:eol-style=LF
+*.txt = svn:mime-type=text/plain; svn:eol-style = native
+EOF
+}
+
+test_expect_success 'initialize git-svn' '
+ mkdir import &&
+ cd import &&
+ echo foo > foo &&
+ svn import -m "import for git-svn" . "$svnrepo" >/dev/null &&
+ cd .. &&
+ rm -rf import &&
+ git-svn init "$svnrepo"
+ git-svn fetch'
+
+test_expect_success 'enable auto-props config' '
+ cd "$gittestrepo" &&
+ mkdir user &&
+ generate_auto_props yes > user/config
+ '
+
+test_expect_success 'add files matching auto-props' '
+ cd "$gittestrepo" &&
+ echo "#!/bin/sh" > exec1.sh &&
+ chmod +x exec1.sh &&
+ echo "hello" > hello.txt &&
+ echo bar > bar &&
+ git add exec1.sh hello.txt bar &&
+ git commit -m "files for enabled auto-props" &&
+ git svn dcommit --config-dir=user
+ '
+
+test_expect_success 'disable auto-props config' '
+ cd "$gittestrepo" &&
+ generate_auto_props no > user/config
+ '
+
+test_expect_success 'add files matching disabled auto-props' '
+ cd "$gittestrepo" &&
+ echo "#!/bin/sh" > exec2.sh &&
+ chmod +x exec2.sh &&
+ echo "world" > world.txt &&
+ echo zot > zot &&
+ git add exec2.sh world.txt zot &&
+ git commit -m "files for disabled auto-props" &&
+ git svn dcommit --config-dir=user
+ '
+
+test_expect_success 'check resulting svn repository' '
+ mkdir work &&
+ cd work &&
+ svn co "$svnrepo" &&
+ cd svnrepo &&
+
+ # Check properties from first commit.
+ test "x$(svn propget svn:executable exec1.sh)" = "x*" &&
+ test "x$(svn propget svn:mime-type exec1.sh)" = \
+ "xapplication/x-shellscript" &&
+ test "x$(svn propget svn:mime-type hello.txt)" = "xtext/plain" &&
+ test "x$(svn propget svn:eol-style hello.txt)" = "xnative" &&
+ test "x$(svn propget svn:mime-type bar)" = "x" &&
+
+ # Check properties from second commit.
+ test "x$(svn propget svn:executable exec2.sh)" = "x*" &&
+ test "x$(svn propget svn:mime-type exec2.sh)" = "x" &&
+ test "x$(svn propget svn:mime-type world.txt)" = "x" &&
+ test "x$(svn propget svn:eol-style world.txt)" = "x" &&
+ test "x$(svn propget svn:mime-type zot)" = "x"
+ '
+
+test_done
^ permalink raw reply related
* Re: [PATCH] Avoid warning when From: is encoded
From: Sverre Rabbelier @ 2008-07-25 16:01 UTC (permalink / raw)
To: Abhijit Menon-Sen; +Cc: Peter Valdemar Mørch, git
In-Reply-To: <20080725131625.GA11221@toroid.org>
On Fri, Jul 25, 2008 at 15:16, Abhijit Menon-Sen <ams@toroid.org> wrote:
> At 2008-07-25 15:06:48 +0200, 4ux6as402@sneakemail.com wrote:
>> Are there any form problems with this patch?
Usually the "comment" part is indented by at least one level, but
afaik that's just etiquette and is not mandatory.
> The patch itself also looks good to me (but I'm not sure if that means I
> should add an Acked-by: line to this message).
Acked-by is reserved for people who are "owners" of the area the patch
touches. So for example, a patch to git-gui could be Acked-by Shawn O.
Pierce, or one related to pack format by Nico (I think?). So you
should Ack it if you have done (a lot of) work in the same area as the
patch before and if the patch looks good.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [EGIT] Supported Eclipse version
From: Shawn O. Pearce @ 2008-07-25 16:16 UTC (permalink / raw)
To: Marek Zawirski; +Cc: Jean-Frannnois Veillette, git, Robin Rosenberg
In-Reply-To: <4889DF49.3000903@gmail.com>
Marek Zawirski <marek.zawirski@gmail.com> wrote:
> Jean-François Veillette wrote:
>>> Maybe some users (or developers) from mailing list can tell us about
>>> their opinion?
>>
>> I think keeping comptability for one version behind the 'current'
>> (still compatible 3.3 while 3.4 is the current) is a reasonable goal.
>> 3.2 is relatively far behind, tools vendor had time to get up to date
>> by now.
>
> Robin, Shawn, would you accept next patches using 3.3 API?
Yes, we should be dropping support for 3.2 now and supporting only
3.3 and 3.4 going forward. 3.2 is ancient and anyone who is serious
aboug using Git with Eclipse really should be on a more current
version of the tools.
Same argument that Junio gave about not supporting Debian users who
are still installing git 1.4.4.4 and hope it works; we suggest they
install the 1.5.x based backport as soon as possible.
Commercial Eclipse distributions that are still based upon 3.2
should be upgrading themselves to 3.3 real soon, and if not, their
users should be forching them to do it.
--
Shawn.
^ permalink raw reply
* [PATCH 7/9 - v2] builtin-checkout-index.c: use parse_options()
From: Michele Ballabio @ 2008-07-25 16:25 UTC (permalink / raw)
To: rene.scharfe; +Cc: sverre, Johannes.Schindelin, git, gitster
In-Reply-To: <48899657.5090209@lsrfire.ath.cx>
This changes "struct checkout" (now it uses ints and not bitfields) to
simplify the parsing code.
Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
On Friday 25 July 2008, René Scharfe wrote:
> In the case of struct checkout, though, we could simply make the
> bitfield members full ints, because there are only a few instances of
> this structure in memory at any given time. Wasting a few bytes of RAM
> in order to gain much simpler code is OK in this case, I think.
> OPT_BOOLEAN looks a lot nicer than a callback.
Yes. I only wanted the changes to be minimal, and only affect the option
parsing. In this sense, I still think the old patch is better. Here it is
the one you suggested (maybe Johannes suggested the same, but I didn't
understand :).
builtin-checkout-index.c | 113 +++++++++++++++++++---------------------------
cache.h | 8 ++--
2 files changed, 50 insertions(+), 71 deletions(-)
diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c
index 71ebabf..135348e 100644
--- a/builtin-checkout-index.c
+++ b/builtin-checkout-index.c
@@ -40,6 +40,7 @@
#include "cache.h"
#include "quote.h"
#include "cache-tree.h"
+#include "parse-options.h"
#define CHECKOUT_ALL 4
static int line_termination = '\n';
@@ -153,18 +154,43 @@ static void checkout_all(const char *prefix, int prefix_length)
exit(128);
}
-static const char checkout_cache_usage[] =
-"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...";
+static const char * const checkout_cache_usage[] = {
+ "git checkout-index [options] [--] <file>...",
+ NULL
+};
static struct lock_file lock_file;
int cmd_checkout_index(int argc, const char **argv, const char *prefix)
{
- int i;
int newfd = -1;
int all = 0;
int read_from_stdin = 0;
int prefix_length;
+ char *stage = NULL;
+
+ const struct option options[] = {
+ OPT_BOOLEAN('a', "all", &all,
+ "checks out all files in the index"),
+ OPT_BOOLEAN('f', "force", &state.force,
+ "force overwrite of existing files"),
+ OPT__QUIET(&state.quiet),
+ OPT_SET_INT('n', "no-create", &state.not_new,
+ "do not checkout new files, refresh existing ones", 1),
+ OPT_BOOLEAN('u', "index", &state.refresh_cache,
+ "update stat information in the index"),
+ OPT_SET_INT('z', NULL, &line_termination,
+ "separate paths with NUL", 0),
+ OPT_BOOLEAN(0, "stdin", &read_from_stdin,
+ "read paths from stdin"),
+ OPT_BOOLEAN(0, "temp", &to_tempfile,
+ "write content to temporary files"),
+ OPT_STRING(0, "prefix", &state.base_dir, "string",
+ "prepend <string> when creating files"),
+ OPT_STRING(0, "stage", &stage, "1|2|3|all",
+ "copy out files from the named stage"),
+ OPT_END()
+ };
git_config(git_default_config, NULL);
state.base_dir = "";
@@ -174,71 +200,24 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
die("invalid cache");
}
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
+ argc = parse_options(argc, argv, options, checkout_cache_usage, 0);
- if (!strcmp(arg, "--")) {
- i++;
- break;
- }
- if (!strcmp(arg, "-a") || !strcmp(arg, "--all")) {
- all = 1;
- continue;
- }
- if (!strcmp(arg, "-f") || !strcmp(arg, "--force")) {
- state.force = 1;
- continue;
- }
- if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet")) {
- state.quiet = 1;
- continue;
- }
- if (!strcmp(arg, "-n") || !strcmp(arg, "--no-create")) {
- state.not_new = 1;
- continue;
- }
- if (!strcmp(arg, "-u") || !strcmp(arg, "--index")) {
- state.refresh_cache = 1;
- if (newfd < 0)
- newfd = hold_locked_index(&lock_file, 1);
- continue;
- }
- if (!strcmp(arg, "-z")) {
- line_termination = 0;
- continue;
- }
- if (!strcmp(arg, "--stdin")) {
- if (i != argc - 1)
- die("--stdin must be at the end");
- read_from_stdin = 1;
- i++; /* do not consider arg as a file name */
- break;
- }
- if (!strcmp(arg, "--temp")) {
+ if ((state.refresh_cache) && (newfd < 0))
+ newfd = hold_locked_index(&lock_file, 1);
+ if (state.base_dir)
+ state.base_dir_len = strlen(state.base_dir);
+
+ if (stage) {
+ if (!strcmp(stage, "all")) {
to_tempfile = 1;
- continue;
- }
- if (!prefixcmp(arg, "--prefix=")) {
- state.base_dir = arg+9;
- state.base_dir_len = strlen(state.base_dir);
- continue;
- }
- if (!prefixcmp(arg, "--stage=")) {
- if (!strcmp(arg + 8, "all")) {
- to_tempfile = 1;
- checkout_stage = CHECKOUT_ALL;
- } else {
- int ch = arg[8];
- if ('1' <= ch && ch <= '3')
- checkout_stage = arg[8] - '0';
- else
- die("stage should be between 1 and 3 or all");
- }
- continue;
+ checkout_stage = CHECKOUT_ALL;
+ } else {
+ int ch = stage[0];
+ if ('1' <= ch && ch <= '3')
+ checkout_stage = stage[0] - '0';
+ else
+ die("stage should be between 1 and 3 or all");
}
- if (arg[0] == '-')
- usage(checkout_cache_usage);
- break;
}
if (state.base_dir_len || to_tempfile) {
@@ -253,8 +232,8 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
}
/* Check out named files first */
- for ( ; i < argc; i++) {
- const char *arg = argv[i];
+ while (argc-- > 0) {
+ const char *arg = *argv++;
const char *p;
if (all)
diff --git a/cache.h b/cache.h
index 38985aa..0bbe33b 100644
--- a/cache.h
+++ b/cache.h
@@ -618,10 +618,10 @@ extern const char *fmt_name(const char *name, const char *email);
struct checkout {
const char *base_dir;
int base_dir_len;
- unsigned force:1,
- quiet:1,
- not_new:1,
- refresh_cache:1;
+ int force;
+ int quiet;
+ int not_new;
+ int refresh_cache;
};
extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
--
1.5.6.3
^ permalink raw reply related
* Re: [PATCH] bash completion: Add long options for 'git describe'
From: Shawn O. Pearce @ 2008-07-25 16:20 UTC (permalink / raw)
To: Thomas Rast; +Cc: SZEDER GGGbor, git, gitster
In-Reply-To: <20080725131532.GB6701@neumann>
SZEDER GGGbor <szeder@ira.uka.de> wrote:
> On Fri, Jul 25, 2008 at 12:02:50PM +0200, Thomas Rast wrote:
> > ---
> Signed off by?
>
> > _git_describe ()
> > {
> > + __git_has_doubledash && return
> This line is superfluous, because 'git describe' does not have any path
> arguments.
Yup. Aside from the two items described above (mising SOB line
and the unnecessary double dash test) this patch looks fine.
--
Shawn.
^ permalink raw reply
* [PATCH 2/2] editor.c: Libify launch_editor()
From: Stephan Beyer @ 2008-07-25 16:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin, Stephan Beyer
In-Reply-To: <1217003322-10291-1-git-send-email-s-beyer@gmx.net>
This patch removes exit()/die() calls and builtin-specific messages
from launch_editor(), so that it can be used as a general libgit.a
function to launch an editor.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
builtin-commit.c | 6 +++++-
builtin-tag.c | 6 +++++-
editor.c | 24 ++++++++++++------------
foo | 1 +
strbuf.h | 2 +-
5 files changed, 24 insertions(+), 15 deletions(-)
create mode 100644 foo
diff --git a/builtin-commit.c b/builtin-commit.c
index 6a9dc0e..9a11ca0 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -646,7 +646,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
char index[PATH_MAX];
const char *env[2] = { index, NULL };
snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
- launch_editor(git_path(commit_editmsg), NULL, env);
+ if (launch_editor(git_path(commit_editmsg), NULL, env)) {
+ fprintf(stderr,
+ "Please supply the message using either -m or -F option.\n");
+ exit(1);
+ }
}
if (!no_verify &&
diff --git a/builtin-tag.c b/builtin-tag.c
index 219f51d..325b1b2 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -295,7 +295,11 @@ static void create_tag(const unsigned char *object, const char *tag,
write_or_die(fd, tag_template, strlen(tag_template));
close(fd);
- launch_editor(path, buf, NULL);
+ if (launch_editor(path, buf, NULL)) {
+ fprintf(stderr,
+ "Please supply the message using either -m or -F option.\n");
+ exit(1);
+ }
unlink(path);
free(path);
diff --git a/editor.c b/editor.c
index 483b62d..eebc3e9 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
{
const char *editor, *terminal;
@@ -15,12 +15,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
editor = getenv("EDITOR");
terminal = getenv("TERM");
- if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
- fprintf(stderr,
- "Terminal is dumb but no VISUAL nor EDITOR defined.\n"
- "Please supply the message using either -m or -F option.\n");
- exit(1);
- }
+ if (!editor && (!terminal || !strcmp(terminal, "dumb")))
+ return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
if (!editor)
editor = "vi";
@@ -28,6 +24,7 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
+ int failed;
const char *args[6];
struct strbuf arg0;
@@ -43,14 +40,17 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
args[i++] = path;
args[i] = NULL;
- if (run_command_v_opt_cd_env(args, 0, NULL, env))
- die("There was a problem with the editor %s.", editor);
+ failed = run_command_v_opt_cd_env(args, 0, NULL, env);
strbuf_release(&arg0);
+ if (failed)
+ return error("There was a problem with the editor '%s'.",
+ editor);
}
if (!buffer)
- return;
+ return 0;
if (strbuf_read_file(buffer, path, 0) < 0)
- die("could not read message file '%s': %s",
- path, strerror(errno));
+ return error("could not read file '%s': %s",
+ path, strerror(errno));
+ return 0;
}
diff --git a/foo b/foo
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/foo
@@ -0,0 +1 @@
+
diff --git a/strbuf.h b/strbuf.h
index 0c6ffad..eba7ba4 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -123,6 +123,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
extern int strbuf_getline(struct strbuf *, FILE *, int);
extern void stripspace(struct strbuf *buf, int skip_comments);
-extern void launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
+extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
#endif /* STRBUF_H */
--
1.6.0.rc0.102.ga1791
^ permalink raw reply related
* [PATCH 1/2] Move launch_editor() from builtin-tag.c to editor.c
From: Stephan Beyer @ 2008-07-25 16:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin, Stephan Beyer
In-Reply-To: <alpine.DEB.1.00.0807251636140.11976@eeepc-johanness>
launch_editor() is declared in strbuf.h but defined in builtin-tag.c.
This patch moves launch_editor() into a new source file editor.c,
but keeps the declaration in strbuf.h.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
Johannes Schindelin wrote:
> > This means I should send them again?
>
> If you want to be nice to the maintainer, yes. If not, no.
I understand ;-)
No need to play the bad guy, so here's the patchset again.
To be kind to the maintainer, I've also run the test suite again,
all tests passed except t4116*.sh, but this is not my fault.
It's the fault of 9a885fac.
Regards,
Stephan
Makefile | 1 +
builtin-tag.c | 53 -----------------------------------------------------
editor.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+), 53 deletions(-)
create mode 100644 editor.c
diff --git a/Makefile b/Makefile
index b01cf1c..df5b0d0 100644
--- a/Makefile
+++ b/Makefile
@@ -410,6 +410,7 @@ LIB_OBJS += diff-no-index.o
LIB_OBJS += diff-lib.o
LIB_OBJS += diff.o
LIB_OBJS += dir.o
+LIB_OBJS += editor.o
LIB_OBJS += entry.o
LIB_OBJS += environment.o
LIB_OBJS += exec_cmd.o
diff --git a/builtin-tag.c b/builtin-tag.c
index c2cca6c..219f51d 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -23,59 +23,6 @@ static const char * const git_tag_usage[] = {
static char signingkey[1000];
-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
-{
- const char *editor, *terminal;
-
- editor = getenv("GIT_EDITOR");
- if (!editor && editor_program)
- editor = editor_program;
- if (!editor)
- editor = getenv("VISUAL");
- if (!editor)
- editor = getenv("EDITOR");
-
- terminal = getenv("TERM");
- if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
- fprintf(stderr,
- "Terminal is dumb but no VISUAL nor EDITOR defined.\n"
- "Please supply the message using either -m or -F option.\n");
- exit(1);
- }
-
- if (!editor)
- editor = "vi";
-
- if (strcmp(editor, ":")) {
- size_t len = strlen(editor);
- int i = 0;
- const char *args[6];
- struct strbuf arg0;
-
- strbuf_init(&arg0, 0);
- if (strcspn(editor, "$ \t'") != len) {
- /* there are specials */
- strbuf_addf(&arg0, "%s \"$@\"", editor);
- args[i++] = "sh";
- args[i++] = "-c";
- args[i++] = arg0.buf;
- }
- args[i++] = editor;
- args[i++] = path;
- args[i] = NULL;
-
- if (run_command_v_opt_cd_env(args, 0, NULL, env))
- die("There was a problem with the editor %s.", editor);
- strbuf_release(&arg0);
- }
-
- if (!buffer)
- return;
- if (strbuf_read_file(buffer, path, 0) < 0)
- die("could not read message file '%s': %s",
- path, strerror(errno));
-}
-
struct tag_filter {
const char *pattern;
int lines;
diff --git a/editor.c b/editor.c
new file mode 100644
index 0000000..483b62d
--- /dev/null
+++ b/editor.c
@@ -0,0 +1,56 @@
+#include "cache.h"
+#include "strbuf.h"
+#include "run-command.h"
+
+void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+{
+ const char *editor, *terminal;
+
+ editor = getenv("GIT_EDITOR");
+ if (!editor && editor_program)
+ editor = editor_program;
+ if (!editor)
+ editor = getenv("VISUAL");
+ if (!editor)
+ editor = getenv("EDITOR");
+
+ terminal = getenv("TERM");
+ if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
+ fprintf(stderr,
+ "Terminal is dumb but no VISUAL nor EDITOR defined.\n"
+ "Please supply the message using either -m or -F option.\n");
+ exit(1);
+ }
+
+ if (!editor)
+ editor = "vi";
+
+ if (strcmp(editor, ":")) {
+ size_t len = strlen(editor);
+ int i = 0;
+ const char *args[6];
+ struct strbuf arg0;
+
+ strbuf_init(&arg0, 0);
+ if (strcspn(editor, "$ \t'") != len) {
+ /* there are specials */
+ strbuf_addf(&arg0, "%s \"$@\"", editor);
+ args[i++] = "sh";
+ args[i++] = "-c";
+ args[i++] = arg0.buf;
+ }
+ args[i++] = editor;
+ args[i++] = path;
+ args[i] = NULL;
+
+ if (run_command_v_opt_cd_env(args, 0, NULL, env))
+ die("There was a problem with the editor %s.", editor);
+ strbuf_release(&arg0);
+ }
+
+ if (!buffer)
+ return;
+ if (strbuf_read_file(buffer, path, 0) < 0)
+ die("could not read message file '%s': %s",
+ path, strerror(errno));
+}
--
1.6.0.rc0.102.ga1791
^ permalink raw reply related
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