* Re: [PATCH] grep: --full-tree
From: James Pickens @ 2009-11-26 18:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Wincent Colaiuta, Jeff King, git
In-Reply-To: <7vd436p339.fsf@alter.siamese.dyndns.org>
On Wed, Nov 25, 2009 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> - git does not accept paths (it lets you specify patterns that match,
> e.g. t/ to name ptahs under t/ directory).
That's not entirely true, unfortunately:
$ echo >> unpack-trees.c
$ git diff --name-status unpack-trees.c
M unpack-trees.c
$ git diff --name-status $PWD/unpack-trees.c
M unpack-trees.c
$ git diff --name-status $PWD/../git/unpack-trees.c
M unpack-trees.c
$ git diff --name-status ../git/unpack-trees.c
fatal: '../git/unpack-trees.c' is outside repository
So it seems that 'git diff' accepts absolute paths as long as they end up
in the repository, but oddly enough, doesn't do so for relative paths.
It's possible that some users have scripts that use absolute paths, and
changing the interpretation would break those scripts. Such scripts
*should* be rare, so maybe it's ok to break them, but it needs to be
considered.
James
^ permalink raw reply
* Re: [PATCH (resend)] Let core.excludesfile default to ~/.gitexcludes.
From: Junio C Hamano @ 2009-11-26 18:18 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Matthieu Moy, git
In-Reply-To: <4B0E6DC9.3070105@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> I'm sorry to jump in so late, and this may sound like bike-shedding, but
> right now we have
>
> .git/info/exclude
> .gitignore
The current dir.c::setup_standard_excludes() implementation reads exclude
patterns from "$GIT_DIR/info/exclude".
As to the way forward regarding "info/exclude", my preference is to teach
the codepath to check if "info/ignore" exists, and read that after it
reads from "info/exclude" (to give the new file a slightly higher
precedence if both exist).
Document that we read from both info/ignore and info/exclude immediately
when this change happens. The updated documentation probably should say
that "info/exclude" is still read and will be read forever, and tell the
readers that there is no difference in functionality between the two,
other than that "info/ignore" is a name that is more consistent with the
".gitignore" (which is the name of in-tree exclude patterns file).
And stop there.
While there is no reason to encourage new repositories to use info/exclude
over info/ignore, we shouldn't stop mentioning info/exclude or declare
deprecation in the documentation, as doing so would:
(1) scare existing users that their info/exclude file may become invalid
some day; and
(2) puzzle new users (who inherited an already initialized repository
that has been maintained by somebody else from the era before this
change) what the info/exclude file in the repository is doing.
> and this would add
>
> ~/.gitexcludes
>
> That is, three terms, or two, where one comes in two variations
> (exclude/exludes). I always wondered why we have two.
>
> The reason for .gitignore is most probably the similarity to
> .${othervcs}ignore, and that is a valid reason.
I try not to get involved in bike-shedding (I removed myself from Cc), but
the events happened in the opposite order.
Originally we had the concept of "exclude patterns", in 9ff768e ([PATCH]
Give show-files the ability to process exclusion pattern., 2005-04-28).
This origin is still visible in the names of command line options to
"ls-files". It was just a flat text file that is not tied to any
particular directory in the work tree.
Then much later in f87f949 (git-ls-files: --exclude mechanism updates.,
2005-07-24) the mechanism was extended, so that we could add per-directory
exclude patterns. But that was just the _ability_; its actual use in the
Porcelains had to wait until ba966b9 (Teach git-status-script about
git-ls-files --others, 2005-08-26), and the commit finally established the
name ".gitignore" that is an in-tree, per-directory file that has exclude
patterns. The name was inconsistent with the established concept of
"excluding", but was chosen to be similar to .scmignore.
Hopefully that would clear your wondering.
^ permalink raw reply
* Re: [PATCH/RFC 02/11] strbuf: add non-variadic function strbuf_vaddf()
From: Junio C Hamano @ 2009-11-26 18:46 UTC (permalink / raw)
To: kusmabite; +Cc: msysgit, git, dotzenlabs, Alex Riesen
In-Reply-To: <40aa078e0911260238rd0c90cag126709d1de5f50de@mail.gmail.com>
Erik Faye-Lund <kusmabite@googlemail.com> writes:
> On Thu, Nov 26, 2009 at 1:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Erik Faye-Lund <kusmabite@googlemail.com> writes:
>>
>>> +void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
>>> {
>>> int len;
>>>
>>> if (!strbuf_avail(sb))
>>> strbuf_grow(sb, 64);
>>> len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
>>> if (len < 0)
>>> die("your vsnprintf is broken");
>>> if (len > strbuf_avail(sb)) {
>>> strbuf_grow(sb, len);
>>> len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
>>> if (len > strbuf_avail(sb)) {
>>> die("this should not happen, your snprintf is broken");
>>> }
>>
>> Hmm, I would have expected to see va_copy() somewhere in the patch text.
>> Is it safe to reuse ap like this in two separate invocations of
>> vsnprintf()?
>
> I think your expectation is well justified, this seems to be a
> portability-bug waiting to happen. Sorry for missing this prior to
> sending out - on Windows this is known to work, and this function is
> currently only used from the Windows implementation of syslog.
>
> How kosher is it to use va_copy in the git-core, considering that it's
> C99? A quick grep reveals only one occurrence of va_copy in the
> source, and that's in compat/winansi.c. Searching the history of next
> reveals that Alex Riesen (CC'd) already removed one occurrence
> (4bf5383), so I'm starting to get slightly scared it might not be OK.
We tend to avoid C99 features and it saved us in a few occasions. Recent
MSVC port revealed that we still had a handful of decl-after-statments but
luckily the necessary fix-ups were minimal because I have been reasonably
careful to reject patches that add it long before MSVC port happened.
> In practice it seems that something like the following works
> portably-enough for many applications, dunno if it's something we'll
> be happy with:
> #ifndef va_copy
> #define va_copy(a,b) ((a) = (b))
> #endif
Since an obvious implementation of va_list would be to make it a pointer
into the stack frame, doing the above would work on many systems. On
esoteric systems that needs something different (e.g. where va_list is
implemented as a size-1 array of pointers, va_copy(a,b) needs to be an
assignment (*(a) = *(b))), people can add compatibility macro later.
Historically some systems that do have a suitable implementation had it
under the name __va_copy() instead, so it would have been better to define
it as something like:
#ifndef va_copy
# ifdef __va_copy
# define va_copy(a,b) __va_copy(a,b)
# else
# /* fallback for the most obvious implementation of va_list */
# define va_copy(a,b) ((a) = (b))
# endif
#endif
But I do not know it still matters in practice anymore.
^ permalink raw reply
* Re: [RFC/PATCH 2/2] status -s: obey color.status
From: Junio C Hamano @ 2009-11-26 19:00 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Michael J Gruber, git, Junio C Hamano
In-Reply-To: <4B0EA06A.1050101@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> Michael J Gruber schrieb:
>> * Is there any policy regarding use of putchar/puts vs. printf?
>
> If the printed string contains color escapes that should be obeyed, you
> can use only fputs, printf, and fprintf. You should not use puts or putchar.
This msysgit-imposed restriction is something even I do not remember
offhand. Could you please document it somewhere in a file any developer
would get by checking out the 'master' branch of git.git?
^ permalink raw reply
* [PATCH] send-email: automatic envelope sender
From: Felipe Contreras @ 2009-11-26 19:04 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
This adds the option to specify the envelope sender as "auto" which
would pick the 'from' address. This is good because now we can specify
the address only in one place (and change it easily).
Comments by Junio C Hamano.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Documentation/git-send-email.txt | 7 ++++---
git-send-email.perl | 6 +++++-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index c85d7f4..8c482f4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -108,9 +108,10 @@ Sending
--envelope-sender=<address>::
Specify the envelope sender used to send the emails.
This is useful if your default address is not the address that is
- subscribed to a list. If you use the sendmail binary, you must have
- suitable privileges for the -f parameter. Default is the value of
- the 'sendemail.envelopesender' configuration variable; if that is
+ subscribed to a list. In order to use the 'From' address, set the
+ value to "auto". If you use the sendmail binary, you must have
+ suitable privileges for the -f parameter. Default is the value of the
+ 'sendemail.envelopesender' configuration variable; if that is
unspecified, choosing the envelope sender is left to your MTA.
--smtp-encryption=<encryption>::
diff --git a/git-send-email.perl b/git-send-email.perl
index 4f5da4e..da2e56e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -862,7 +862,11 @@ X-Mailer: git-send-email $gitversion
my @sendmail_parameters = ('-i', @recipients);
my $raw_from = $sanitized_sender;
- $raw_from = $envelope_sender if (defined $envelope_sender);
+ if (defined $envelope_sender) {
+ if (not $envelope_sender eq "auto") {
+ $raw_from = $envelope_sender;
+ }
+ }
$raw_from = extract_valid_address($raw_from);
unshift (@sendmail_parameters,
'-f', $raw_from) if(defined $envelope_sender);
--
1.6.6.rc0.61.geeb75
^ permalink raw reply related
* [PATCH 0/2] format-patch: fix dashdash
From: Felipe Contreras @ 2009-11-26 19:11 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras
This doesn't work
git format-patch <committish> -- <path not in working dir>
So here are some fixes and a test.
Felipe Contreras (2):
format-patch: fix dashdash usage
format-patch: add test for dashdash
builtin-log.c | 3 ++-
t/t4014-format-patch.sh | 4 ++++
2 files changed, 6 insertions(+), 1 deletions(-)
^ permalink raw reply
* [PATCH 1/2] format-patch: fix dashdash usage
From: Felipe Contreras @ 2009-11-26 19:11 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras
In-Reply-To: <1259262720-24077-1-git-send-email-felipe.contreras@gmail.com>
Otherwise 'git format-patch <committish> -- <non-existent-path>' doesn't
work.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-log.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 33fa6ea..1766349 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -976,7 +976,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
*/
argc = parse_options(argc, argv, prefix, builtin_format_patch_options,
builtin_format_patch_usage,
- PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
+ PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+ PARSE_OPT_KEEP_DASHDASH);
if (do_signoff) {
const char *committer;
--
1.6.6.rc0.61.geeb75
^ permalink raw reply related
* [PATCH 2/2] format-patch: add test for dashdash
From: Felipe Contreras @ 2009-11-26 19:12 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras
In-Reply-To: <1259262720-24077-1-git-send-email-felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4014-format-patch.sh | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7f267f9..d5b002d 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -536,6 +536,10 @@ test_expect_success 'format-patch --signoff' '
grep "^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
'
+test_expect_success 'format-patch -- <path>' '
+ git format-patch master..side -- file
+'
+
echo "fatal: --name-only does not make sense" > expect.name-only
echo "fatal: --name-status does not make sense" > expect.name-status
echo "fatal: --check does not make sense" > expect.check
--
1.6.6.rc0.61.geeb75
^ permalink raw reply related
* Re: [RFC/PATCHv8 08/10] fast-import: Proper notes tree manipulation using the notes API
From: Shawn O. Pearce @ 2009-11-26 19:33 UTC (permalink / raw)
To: Johan Herland; +Cc: git, gitster
In-Reply-To: <200911261210.48138.johan@herland.net>
Johan Herland <johan@herland.net> wrote:
> BTW, while we're on the topic, this whole code is only present because I
> assume it's not possible to edit the fast-import tree structure _while_
> traversing it. Is this assumption correct, or are there ways to get
> around maintaining a separate edit list that is applied to the tree
> structure afterwards?
IIRC you can actually edit the tree while you are walking through it.
You just have to watch out for the fact that a struct tree_content
can be reallocated (and thus moved in memory) if the entry_capacity
was too small for the new entry_count when inserting a new entry.
tree_content_set() handles this in its API by taking a struct
tree_entry* rather than a struct tree_content*. This way if the
tree has to expand during the set and gets reallocated we can return
the new tree pointer to the caller through the struct tree_entry
tree field.
--
Shawn.
^ permalink raw reply
* Re: Merge priority
From: Junio C Hamano @ 2009-11-26 19:47 UTC (permalink / raw)
To: Alex Riesen; +Cc: Howard Miller, git
In-Reply-To: <81b0412b0911260742l4beb2305y87c01dd61d7c2b57@mail.gmail.com>
Alex Riesen <raa.lkml@gmail.com> writes:
> You can take a look at rerere, though. It should help resolving repeating
> conflicts by recording a resolution of your choice.
Use of rerere often helps but there is a caveat.
Even if two branches (e.g. 'master' and 'maint') currently has the same
tree, the desired outcome of merging the same commit into them could be
different depending on the purpose of these branches. You may want to
resolve a conflicted merge into 'maint' more conservatively than the way
you would resolve the exact same merge into 'master'.
Imagine a case where 'master' branch added a better helper functions in
other files, but they are not in 'maint' yet, and the conflicted part
could be resolved better if you made it call these newer helper functions
available only in 'master'.
Unfortunately the conflicts would look exactly the same to rerere, so if
you record the result of the merge to 'master', replaying that resolution
while merging the same commit to 'maint' would produce undesirable result.
As to "does the aggregated result make sense if you blindly add changes in
'stable' for only conflicted parts to an automerge result?", which was
your other point, you may refer your readers to Avery's recent updates to
resurrect -Xours option to "git merge".
^ permalink raw reply
* Re: [PATCH 1/2] format-patch: fix dashdash usage
From: Junio C Hamano @ 2009-11-26 19:57 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <1259262720-24077-2-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Otherwise 'git format-patch <committish> -- <non-existent-path>' doesn't
> work.
Instead of "doesn't work", I really wished you wrote something like:
$ git format-patch <commit> -- <path>
complains that <path> does not exist in the current work tree and the
user needs to explicitly specify "--", even though the user _did_ give
a "--". This is because it incorrectly removes "--" from the command
line arguments that is later passed to setup_revisions().
Remember that you are trying to help somebody who has to write Release
Notes out of "git log" output.
I actually have a bigger question, though. Does it even make sense to
allow pathspecs to format-patch? We sure are currently loose and take
them, but I doubt it is by design.
The patch itself looks good and is a candidate 'maint' material, if the
answer to the above question is a convincing "yes, because ...".
^ permalink raw reply
* Re: [egit-dev] Re: jgit problems for file paths with non-ASCII characters
From: Shawn O. Pearce @ 2009-11-26 19:57 UTC (permalink / raw)
To: Thomas Singer; +Cc: Johannes Schindelin, Marc Strapetz, git
In-Reply-To: <4B0E9F69.9040502@syntevo.com>
Thomas Singer <thomas.singer@syntevo.com> wrote:
> > You mean we should do the same thing as Apple with HFS? Are you serious?
>
> Yes, I'm serious. IMHO there should be a defined clear encoding used for
> files names in the repository. Otherwise you don't know what you can expect
> by reading it - it could mean anything. File names are in fact strings which
> are based on characters. To convert characters to bytes (or visa versa) you
> need to know the encoding.
That's likely not going to fly. HFS+ has changed their decomposition
rules at least once, which means the byte sequence for the same
character sequence would differ, and a tree or commit hash would
come out different depending upon which rules you were following.
See [1] for details on what HFS+ does.
Also, Linus has previously stated HFS+ chose the worst possible
way to encode the names. Getting Linus to admit he was wrong is
impossible, getting Linus to accept the HFS+ encoding rules as the
standard format used in a Git repository is not likely to happen.
Fortunately Linus carries a slightly smaller stick in Git than he
used to, but he is quite vocal and people tend to listen.
[1] http://developer.apple.com/mac/library/technotes/tn/tn1150.html#UnicodeSubtleties
--
Shawn.
^ permalink raw reply
* Re: [egit-dev] Re: jgit problems for file paths with non-ASCII characters
From: Shawn O. Pearce @ 2009-11-26 20:03 UTC (permalink / raw)
To: Marc Strapetz; +Cc: EGit developer discussion, git, robin.rosenberg
In-Reply-To: <4B0E8FF2.8040206@syntevo.com>
Marc Strapetz <marc.strapetz@syntevo.com> wrote:
> > We should try to work harder with the git-core folks to get character
> > set encoding for file names worked out. We might be able to use a
> > configuration setting in the repository to tell us what the proper
> > encoding should be, and if not set, assume UTF-8.
>
> I agree that this should be the ultimate goal, though the default should
> better be "system encoding" for compatibility with current git
> repositories and instead have newer git versions always set encoding to
> UTF-8. Thus, for our jgit clone I've introduced a system property to
> configure Constants.PATH_ENCODING set to system encoding. It's used by
> PathFilter and this resolves my original problem.
That's probably a good point, using the system encoding on a
repository may produce the file names in a more compatible way
with git-core. But we probably don't want the encoding to be a
single encoding constant in this JVM, we probably need to support
a per-repository configuration of the encoding for path names so
that we can eventually move to a non-platform specific encoding.
> I have tried to switch more usages from Constants.CHARACTER_ENCODING to
> Constants.PATH_ENCODING, but ended up in confusion due to my lack of
> understanding: primarily because I couldn't tell anymore whether encoded
> strings were file names or not.
Heh. Yea. There are a number of file name encoding sites. I think
everything in the treewalk package, as well as the GitIndex, Tree and
DirCache* classes. Also the Patch class and its FileHeader friend.
> Does it make sense to explicitly
> distinguish encoding usages in that way? We could try to contribute here
> (and hopefully cause less review effort to jgit developers than the
> changes itself are worth ;-)
Yes, it does. Because we eventually need to support encodings
other than the current UTF-8 we assume for file names, especially
if a repository is using the local filesystem encoding and that
isn't UTF-8.
--
Shawn.
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 00/11] daemon-win32
From: Johannes Sixt @ 2009-11-26 20:04 UTC (permalink / raw)
To: msysgit; +Cc: Erik Faye-Lund, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <1259196260-3064-1-git-send-email-kusmabite@gmail.com>
On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
> This is my stab at cleaning up Mike Pape's patches for git-daemon
> on Windows for submission, plus some of my own.
Please rebase the series on top of current master. I'd appreciate if you could
make it available to pull from somewhere. I've a version here:
git://repo.or.cz/git/mingw/j6t.git ef/mingw-daemon
(including Martin's getaddrinfo updates) but the resulting git-daemon crashes
immediately after there is a connection.
-- Hannes
^ permalink raw reply
* Re: [PATCH (resend)] Let core.excludesfile default to ~/.gitexcludes.
From: David Aguilar @ 2009-11-26 20:07 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Bert Wesarg, Matthieu Moy, git, gitster
In-Reply-To: <4B0E8529.3040609@drmicha.warpmail.net>
On Thu, Nov 26, 2009 at 02:39:53PM +0100, Michael J Gruber wrote:
>
> Now, that is bike shedding ;)
>
> It seems to me that all ~/.*rc that I have are config files (.bashrc,
> .xinitrc...), and all condif subdirs ~/.* are named by the
> program/subsystem (.qt, .kde, .gnupg), which we cannot do any more, and
> which is why I suggested .gitglobal. But I'd be fine with .gitrc.
>
> > On the other
> > hand the --global option to git config specifies the .gitconfig in
> > your HOME.
>
> That would have to change (ouch, ducking). Transition plan would be:
>
> ~/.gitconfig, ~/.gitrc/config::
> User-specific configuration file. Also called "global"
> configuration file. Git looks in these locations in the
> specified order and uses the first one it finds.
>
> $(prefix)/etc/gitconfig, $(prefix)/etc/gitrc/config::
> System-wide configuration file. Git looks in these locations
> in the specified order and uses the first one it finds.
>
> This would mean no surprises for users with existing config, one could
> teach the new preferred locations exclusively, and at some future point
> one could phase out the old paths.
>
> Michael
If we're going to bikeshed then let's throw a standard in there:
http://standards.freedesktop.org/basedir-spec/basedir-spec-0.6.html
~/.gitrc/ doesn't make sense (it's not a file) and ~/.gitglobal/
hurts my eyes.
"global"? Huh? Isn't it really user-specific?
Why not call it ~/.gituser/ then?
And what about the standard?
The silly standard says to use ~/.config/git/.
I'm quite happy with ~/.gitconfig and ~/.gitexclude if that's all
there is to git's per-user configuration abilities, especially
since ~/.gitexclude is less common. _Much_ less common from
what I've seen in practice.
Being that we cannot predict the future then there is some
appeal to a top-leve ~/.config/git/-like directory. But...
Like Junio said, I would stop only after adding support for
the new paths. We don't want to confuse old or new users
and we should never deprecate existing ~/.gitconfig.
So now the "user" config is not just tied to one file but
is instead multiple files? I dunno.. I kinda don't like
that but the only reason is because I'm going to have to
go and change code to take that into account.
When I have to change code for little added benefit
I ask questions.
What about:
$ git config --global foo.bar baz
What file does that touch?
~/.gitconfig or ~/.config/git/config?
What if ~/.gitconfig exists and ~/.config/git/config doesn't?
What about vice versa?
Okay, I also don't like it for that reason.
What if you jump between git versions? Now the previous
question is much more important -- it means that we *must*
write to ~/.gitconfig to keep backwards compatibility otherwise
someone will config something with git-vNew and be surprised
when git-vOld does not find it.
And if we must write to ~/.gitconfig then
why does ~/.config/git/config even exist?
I guess all I'm saying is that I'm quite happy with
~/.gitconfig and do not see a compelling reason as
to why we'd need to transition to a different path.
Yes, I'm being lazy. I don't feel like changing code
when stuff works just fine right now ;-)
And if we were to change it, then what about JGit,
Dulwich, GitSharp, etc? Who's going to change those?
To quote an old famous horse, "No sir, I don't like it."
--
David
^ permalink raw reply
* [RFC/PATCH] gitweb: Make linking to actions requiring JavaScript a feature
From: Jakub Narebski @ 2009-11-26 20:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Boyd, git
In-Reply-To: <200911260159.48311.jnareb@gmail.com>
On Thu, 26 Nov 2009, Jakub Narebski wrote:
> On Thu, 26 Nov 2009, Junio C Hamano wrote:
> > A follow-up patch to add a gitweb configuration switch that disables the
> > non-working view by default but allows site owners to enable it in order
> > to help improving the feature would be a sensible thing to do. As long as
> > that patch is solidly done we can merge the whole thing to 'master' in the
> > upcoming release.
>
> But if it is already in 'next', then I'll try to come up with patch which
> makes JavaScript-ing links (replacing links with JavaScript to equivalent
> actions utilizing JavaScript, currently only 'blame' -> 'blame_incremental')
> configurable.
Here it is. I am a bit ambiguous about *naming* of this feature (and
whether it should be overridable), that's why it is marked as RFC.
Also the subject of this commit could have been better, I think...
-- >8 --
Let gitweb turn some links (like 'blame' links) into linking to
actions which require JavaScript (like 'blame_incremental' action)
only if 'javascript-actions' feature is enabled.
This means that links to such actions would be present only if both
JavaScript is enabled and 'javascript-actions' feature is enabled.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a80cbd3..0ab47e1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -432,6 +432,13 @@ our %feature = (
'timed' => {
'override' => 0,
'default' => [0]},
+
+ # Enable turning some links into links to actions which require
+ # JavaScript to run (like 'blame_incremental'). Enabled by default.
+ # Project specific override is currently not supported.
+ 'javascript-actions' => {
+ 'override' => 0,
+ 'default' => [1]},
);
sub gitweb_get_feature {
@@ -3326,7 +3333,7 @@ sub git_footer_html {
qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
qq! "!. href() .qq!");\n!.
qq!</script>\n!;
- } else {
+ } elsif (gitweb_check_feature('javascript-actions')) {
print qq!<script type="text/javascript">\n!.
qq!window.onload = fixLinks;\n!.
qq!</script>\n!;
--
1.6.5.3
^ permalink raw reply related
* Re: [RFC/PATCH] gitweb: Make linking to actions requiring JavaScript a feature
From: Junio C Hamano @ 2009-11-26 20:34 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Stephen Boyd, git
In-Reply-To: <200911262112.16280.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Let gitweb turn some links (like 'blame' links) into linking to
> actions which require JavaScript (like 'blame_incremental' action)
> only if 'javascript-actions' feature is enabled.
Hmm, instead of "fixLinks" that actually munges existing links to working
action with some other action, can that be "addLinks" that _adds_ new
links to features available only when JS can be used?
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 03/11] mingw: implement syslog
From: Johannes Sixt @ 2009-11-26 21:23 UTC (permalink / raw)
To: msysgit; +Cc: Erik Faye-Lund, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <1259196260-3064-4-git-send-email-kusmabite@gmail.com>
On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
> +void syslog(int priority, const char *fmt, ...) {
Style: Brace goes to next line.
> + struct strbuf msg;
> + va_list va;
> + WORD logtype;
> +
> + strbuf_init(&msg, 0);
> + va_start(va, fmt);
> + strbuf_vaddf(&msg, fmt, va);
> + va_end(va);
I would
const char* arg;
if (strcmp(fmt, "%s"))
die("format string of syslog() not implemented")
va_start(va, fmt);
arg = va_arg(va, char*);
va_end(va);
because we have exactly one invocation of syslog(), which passes "%s" as
format string. Then strbuf_vaddf is not needed. Or even simpler: declare the
function as
void syslog(int priority, const char *fmt, const char*arg);
> + ReportEventA(ms_eventlog,
> + logtype,
> + (WORD)NULL,
> + (DWORD)NULL,
> + NULL,
> + 1,
> + 0,
> + (const char **)&msg.buf,
> + NULL);
Why do you pass NULL and apply casts? The first one gives a warning.
Did you read this paragraph in the documentation?
http://msdn.microsoft.com/en-us/library/aa363679(VS.85).aspx
"Note that the string that you log cannot contain %n, where n is an integer
value (for example, %1) because the event viewer treats it as an insertion
string. ..."
How are the chances that this condition applies to our use of the function?
-- Hannes
^ permalink raw reply
* Re: [RFC/PATCH] gitweb: Make linking to actions requiring JavaScript a feature
From: Jakub Narebski @ 2009-11-26 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Boyd, git
In-Reply-To: <7vws1d3tzk.fsf@alter.siamese.dyndns.org>
Dnia czwartek 26. listopada 2009 21:34, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > Let gitweb turn some links (like 'blame' links) into linking to
> > actions which require JavaScript (like 'blame_incremental' action)
> > only if 'javascript-actions' feature is enabled.
>
> Hmm, instead of "fixLinks" that actually munges existing links to working
> action with some other action, can that be "addLinks" that _adds_ new
> links to features available only when JS can be used?
I am not sure if this would make sense.
Both 'blame' (running "git blame --porcelain") and 'blame_incremental'
(running "git blame --incremental") actions generate *the same* view,
modulo some very minor differences. The idea behind 'blame_incremental'
is that it provides incremental updates to long-running action, and
hopefully also reduce server load, at least a bit.
It might be however good *interim* solution, so people would be able
to test 'blame_incremental' view without having to edit gitweb links.
Hmmm....
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 06/11] run-command: add kill_async() and is_async_alive()
From: Johannes Sixt @ 2009-11-26 21:46 UTC (permalink / raw)
To: msysgit; +Cc: Erik Faye-Lund, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <1259196260-3064-7-git-send-email-kusmabite@gmail.com>
On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
> +int kill_async(struct async *async)
> +{
> +#ifndef WIN32
> + return kill(async->pid, SIGTERM);
> +#else
> + DWORD ret = 0;
> + if (!TerminateThread(async->tid, 0))
> + ret = error("killing thread failed: %lu", GetLastError());
Ugh! Did you read the documentation of TerminateThread()?
We need to kill processes/threads when we detect that there are too many
connections. But TerminateThread() is such a dangerous function that we
cannot pretend that everything is good, and we continue to accept
connections.
Unless we find a different solution, I would prefer to punt and die instead.
> + else if (!GetExitCodeThread(async->tid, &ret))
> + ret = error("cannot get thread exit code: %lu", GetLastError());
What should the exit code be good for? The return value of this function can
only be -1 (failure, could not kill) or 0 (success, process killed).
-- Hannes
^ permalink raw reply
* [PATCH] gitworkflows: Consistently back-quote git commands
From: Björn Gustavsson @ 2009-11-26 21:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
Documentation/gitworkflows.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/gitworkflows.txt b/Documentation/gitworkflows.txt
index 91c0eea..065441d 100644
--- a/Documentation/gitworkflows.txt
+++ b/Documentation/gitworkflows.txt
@@ -229,7 +229,7 @@ To verify that 'master' is indeed a superset of 'maint', use git log:
.Verify 'master' is a superset of 'maint'
[caption="Recipe: "]
=====================================
-git log master..maint
+`git log master..maint`
=====================================
This command should not list any commits. Otherwise, check out
--
1.6.6.rc0.15.g4fa80
^ permalink raw reply related
* Re: [msysGit] [PATCH/RFC 07/11] run-command: support input-fd
From: Johannes Sixt @ 2009-11-26 21:53 UTC (permalink / raw)
To: msysgit; +Cc: Erik Faye-Lund, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <1259196260-3064-8-git-send-email-kusmabite@gmail.com>
On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
> This patch adds the possibility to supply a non-0
> file descriptor for communucation, instead of the
> default-created pipe. The pipe gets duplicated, so
> the caller can free it's handles.
>
> This is usefull for async communication over sockets.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
> run-command.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/run-command.c b/run-command.c
> index e5a0e06..98771ef 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -327,7 +327,10 @@ int start_async(struct async *async)
> {
> int pipe_out[2];
>
> - if (pipe(pipe_out) < 0)
> + if (async->out) {
> + pipe_out[0] = dup(async->out);
> + pipe_out[1] = dup(async->out);
> + } else if (pipe(pipe_out) < 0)
> return error("cannot create pipe: %s", strerror(errno));
> async->out = pipe_out[0];
Hm. If async->out != 0:
pipe_out[0] = dup(async->out);
async->out = pipe_out[0];
This is confusing.
Moreover, you are assigning (a dup of) the same fd to the writable end. This
assumes a bi-directional channel. I don't yet know what I should think about
this (haven't studied the later patches, yet).
It would be great if you could add a few words to
Documentation/technical/api-runcommand.txt.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/8] git-merge-file --ours, --theirs
From: Avery Pennarun @ 2009-11-26 21:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nanako Shiraishi
In-Reply-To: <7vy6ltdd2l.fsf@alter.siamese.dyndns.org>
On Thu, Nov 26, 2009 at 1:17 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Except for parse-optification, this one is more or less a verbatim copy of
> my patch, and I think I probably deserve an in-body "From: " line for this
> [PATCH 1/8], [PATCH 6/8] and [PATCH 8/8] to take the full authorship of
> them.
[...]
> Imagine that Avery were an area expert (the subsystem maintainer) on "git
> merge" and downwards, and somebody who did not know that "merge" has
> already been rewritten in C, nor some parts of the system have been
> rewritten to use parse-options, submitted a patch series for review and
> Avery is helping to polish it up [*1*].
I'm quite open to doing this however you want; I definitely consider
it your patch series. My main measurable contribution is just the
unit tests that I wrote.
However, when thinking about this, I wasn't worried so much about the
correct placement of credit as of discredit. The merge code has
changed sufficiently since you wrote this patch series that every one
of them required quite a lot of conflict resolution. Most of the
conflicts were pretty obvious how to resolve, but it was tedious and
error prone, and there's a reasonably high probability that I screwed
up something while doing so.
I imagined what people would expect to see when they do 'git blame' to
explain the source of a problem. If they see your name, you might be
blamed for my errors; if they see my name with a "based on a patch by
Junio" in the changelog, then I would be (probably correctly) blamed
for the errors, while you can retain credit for the success.
Mostly, however, I didn't want to be sending out patches in your name
that weren't actually done by you. If you'd like me to do so,
however, then I will :)
>> +/* merge favor modes */
>> +#define XDL_MERGE_FAVOR_OURS 0x0010
>> +#define XDL_MERGE_FAVOR_THEIRS 0x0020
>> +#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)
>
> This is a bad change. It forces the high-level layer of the resulting
> code to be aware that the favor bits are shifted by 4 and it is different
> from what the low-level layer expects. If I were porting it to
> parse-options, I would have kept OURS = 1 and THEIRS = 2 as the original
> patch, [...]
Ouch, yes, that wasn't very clear thinking on my part. I meant for
XDL_MERGE_FAVOR(flags) to return either XDL_MERGE_FAVOR_OURS or
XDL_MERGE_FAVOR_THEIRS, but clearly it doesn't. Will fix.
Avery
^ permalink raw reply
* qgit question: tagged commits not on a branch
From: Chris.Cheney @ 2009-11-26 21:54 UTC (permalink / raw)
To: git
My commit graph has a number of forks (I can't use the word "branches"
here) that are referenced only by a tag. Whereas gitk --all displays this
graph including those forks, qgit does not display those forks - I don't
see a way to make it do so, other than by adding branches to those tagged
commits.
Have I overlooked something?
TIA
Chris
^ permalink raw reply
* Re: [PATCH 2/8] builtin-merge.c: call exclude_cmds() correctly.
From: Avery Pennarun @ 2009-11-26 22:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpr75hmpq.fsf@alter.siamese.dyndns.org>
On Thu, Nov 26, 2009 at 12:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Avery Pennarun" <apenwarr@gmail.com> writes:
>
>> We need to call exclude_cmds() after the loop, not during the loop, because
>> excluding a command from the array can change the indexes of objects in the
>> array. The result is that, depending on file ordering, some commands
>> weren't excluded as they should have been.
>
> As an independent bugfix, I would prefer this to be made against 'maint'
> and not as a part of this series.
>
> How did you notice it? Can you make a test case out of your experience of
> noticing this bug in the first place, by the way (I am suspecting that you
> saw some breakage and chased it in the debugger)?
The story behind this one is a bit silly, but since you asked: I
forgot to add recursive-ours and recursive-theirs to the list of known
merge strategies, but was surprised to find that my test for
recursive-theirs passed, while recursive-ours didn't. Investigating
further, I found that the printed list of "found" strategies included
recursive-theirs but not recursive-ours. Turns out that this is
because when recursive-ours was being (correctly) removed, that slot
in the array was being filled by recursive-theirs, and then
immediately i++, which meant that recursive-theirs was never checked
for exclusion as it should have been.
Of course, after fixing this bug *both* tests were broken, but the
correct thing to do was add both strategies to the list, which hides
the effect of this bugfix.
Since the bug is actually that *too many* strategies are listed
instead of too few, it's pretty minor and I doubt it needs to go into
maint. Also, I don't know of a way to test it that would be reliable.
And I doubt this particular bug will recur anyway. (If it were too
*few* strategies listed, I'm guessing it would be caught by any number
of other tests.)
Suggestions welcome.
Thanks,
Avery
^ 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