* [PATCH] Remove empty ref directories while reading loose refs
From: Nguyễn Thái Ngọc Duy @ 2012-02-10 16:25 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Michael Haggerty,
Nguyễn Thái Ngọc Duy
Empty directories in $GIT_DIR/refs increases overhead at startup.
Removing a ref does not remove its parent directories even if it's the
only file left so empty directories will be hanging around.
pack-refs was taught of cleaning up empty directories in be7c6d4
(pack-refs: remove newly empty directories - 2010-07-06), but it only
checks parent directories of packed refs only. Already empty dirs are
left untouched.
This patch removes empty directories as we see while traversing
$GIT_DIR/refs and reverts be7c6d4 because it's no longer needed.
Some directories, even if empty, are not removed:
- refs: this one is needed to recognize a git repository
- refs/heads and refs/tags: these are created by init-db, people may
expect them to always be there
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
I don't think the a few extra rmdir()s from time to time at startup
are going to cause any problems. Making delete_ref() delete empty
directories takes more effort and probably not worth it.
Of course this only works if people do not expect empty directories
to stay in $GIT_DIR/refs permanently. They now may need to put .keep
file in to keep parent directories from being removed. Would anyone
do that?
pack-refs.c | 32 --------------------------------
refs.c | 14 +++++++++++++-
t/t3210-pack-refs.sh | 6 ------
3 files changed, 13 insertions(+), 39 deletions(-)
diff --git a/pack-refs.c b/pack-refs.c
index f09a054..ec9e476 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -60,37 +60,6 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
return 0;
}
-/*
- * Remove empty parents, but spare refs/ and immediate subdirs.
- * Note: munges *name.
- */
-static void try_remove_empty_parents(char *name)
-{
- char *p, *q;
- int i;
- p = name;
- for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */
- while (*p && *p != '/')
- p++;
- /* tolerate duplicate slashes; see check_refname_format() */
- while (*p == '/')
- p++;
- }
- for (q = p; *q; q++)
- ;
- while (1) {
- while (q > p && *q != '/')
- q--;
- while (q > p && *(q-1) == '/')
- q--;
- if (q == p)
- break;
- *q = '\0';
- if (rmdir(git_path("%s", name)))
- break;
- }
-}
-
/* make sure nobody touched the ref, and unlink */
static void prune_ref(struct ref_to_prune *r)
{
@@ -99,7 +68,6 @@ static void prune_ref(struct ref_to_prune *r)
if (lock) {
unlink_or_warn(git_path("%s", r->name));
unlock_ref(lock);
- try_remove_empty_parents(r->name);
}
}
diff --git a/refs.c b/refs.c
index b8843bb..80ebba3 100644
--- a/refs.c
+++ b/refs.c
@@ -343,6 +343,12 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
struct dirent *de;
int baselen = strlen(base);
char *refname = xmalloc(baselen + 257);
+ int empty_dir = 1;
+
+ if (!strcmp(base, "refs") ||
+ !strcmp(base, "refs/heads") ||
+ !strcmp(base, "refs/tags"))
+ empty_dir = 0;
memcpy(refname, base, baselen);
if (baselen && base[baselen-1] != '/')
@@ -355,8 +361,12 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
int namelen;
const char *refdir;
- if (de->d_name[0] == '.')
+ if (de->d_name[0] == '.') {
+ if (de->d_name[1] != '.' || de->d_name[2])
+ empty_dir = 0;
continue;
+ }
+ empty_dir = 0;
namelen = strlen(de->d_name);
if (namelen > 255)
continue;
@@ -387,6 +397,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
}
free(refname);
closedir(dir);
+ if (empty_dir)
+ rmdir(path);
}
}
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index cd04361..5251740 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -60,12 +60,6 @@ test_expect_success 'see if git pack-refs --prune remove ref files' '
! test -f .git/refs/heads/f
'
-test_expect_success 'see if git pack-refs --prune removes empty dirs' '
- git branch r/s/t &&
- git pack-refs --all --prune &&
- ! test -e .git/refs/heads/r
-'
-
test_expect_success \
'git branch g should work when git branch g/h has been deleted' \
'git branch g/h &&
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: git status: small difference between stating whole repository and small subdirectory
From: Piotr Krukowiecki @ 2012-02-10 16:18 UTC (permalink / raw)
To: Git Mailing List; +Cc: Nguyen Thai Ngoc Duy
In-Reply-To: <CAA01Csr8FbvQ8uFvxX8_6i-hysin6JuaifVVC-yoLyoT0N5F4Q@mail.gmail.com>
On Fri, Feb 10, 2012 at 10:42 AM, Piotr Krukowiecki
<piotr.krukowiecki@gmail.com> wrote:
> Hi,
>
> I compared stating whole tree vs one small subdirectory, and I
> expected that for the subdirectory status will be very very fast.
> After all, it has only few files to stat. But it's not fast. Why?
>
>
> With cold cache (echo 3 | sudo tee /proc/sys/vm/drop_caches):
>
> $ time git status > /dev/null
> real 0m41.670s
> user 0m0.980s
> sys 0m2.908s
>
> $ time git status -- src/.../somedir > /dev/null
> real 0m17.380s
> user 0m0.748s
> sys 0m0.328s
>
>
> With warm cache:
>
> $ time git status > /dev/null
> real 0m0.792s
> user 0m0.404s
> sys 0m0.384s
>
> $ time git status -- src/.../somedir > /dev/null
> real 0m0.335s
> user 0m0.288s
> sys 0m0.048s
>
>
> Repository/dir stats:
>
> $ find * -type f | wc -l
> 127189
> $ du -shc * | grep total
> 2.2G total
>
> $ find src/.../somedir -type f | wc -l
> 55
> $ du -shc src/.../somedir | grep total
> 224K total
>
>
> $ git --version
> git version 1.7.9.rc0.10.gbeecc
I can't reproduce this behavior at the moment. 'status' on the
directory takes about 1.5s instead of 17s. status on whole repository
takes 27s.
This is my work repository, so it was changed today.
I'll get back to you when I can reproduce the problem again...
--
Piotr Krukowiecki
^ permalink raw reply
* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Jakub Narebski @ 2012-02-10 14:55 UTC (permalink / raw)
To: Michał Kiedrowicz; +Cc: git
In-Reply-To: <20120210151528.56145e0c@gmail.com>
On Fri, 10 Feb 2012, Michał Kiedrowicz wrote:
> Jakub Narebski <jnareb@gmail.com> wrote:
> > Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:
> >
> > > The code that comares lines is based on
> > > contrib/diff-highlight/diff-highlight, except that it works with
> > > multiline changes too. It also won't highlight lines that are
> > > completely different because that would only make the output unreadable.
> > > Combined diffs are not supported but a following commit will change it.
> >
> > I was thinking that if gitweb were to support "diff refinement
> > highlighting", it would either use one of *Diff* packages from CPAN,
> > or "git diff --word-diff" output.
>
> I think highlighting inline and side-by-side diff outputs is
> something different from "git diff --word-diff". I find it useful for
> people who are used to these diff formats (i.e. me :).
I was thinking about *using* "git diff --word-diff" for diff refinement
highlighting of inline (unified) and side-by-side diff...
though having an option of showing word-diff would be I think a good
idea in some cases, like e.g. documentation changes.
> OTOH I'm not against using a dedicated package from CPAN. But I think
> my approach is proven to work (I use contrib/diff-highlight as a
> filter) and more lightweight (doesn't add another dependency to
> gitweb). Moreover, adding support for some Diff package may be done
> later, at any moment. It's just a matter of replacing one function
> (format_rem_add_line()) with the one that uses Diff.
O.K., if it is tested code, then all is good. Well, except the fact
that I'm rather wary about adding more code to gitweb when it is still
single monolithic script, rather than split into packages.
Anyway, I'll try to review those patches soon. I like the refactoring
work (that is from what I had chance to examine).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v2 28/51] refs.c: rename ref_array -> ref_dir
From: Michael Haggerty @ 2012-02-10 14:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <4F158E99.2020906@alum.mit.edu>
On 01/17/2012 04:07 PM, Michael Haggerty wrote:
> On 12/14/2011 12:24 AM, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>> ... But there are so many calls to the
>>> for_each_ref*() family of functions that I wasn't able to determine
>>> exactly which should allow for extra refs and which shouldn't.
>>
>> Only the ones that follow add_extra_ref() in the control flow.
>>
>> builtin/clone.c adds them in setup_reference() to deal with --reference.
>> The objects known to be complete in these repositories we borrow from
>> need to be marked complete on our end (i.e. clone does not have to fetch)
>> and transport_fetch_refs() that eventually goes to fetch_refs_via_pack()
>> that calls fetch_pack() uses this information. All three for_each_ref()
>> calls in builtin/fetch-pack.c are about "what are the objects that we know
>> are complete?" and needs to pay attention to extra refs.
>>
>> Having said that, I have a slight suspicion that you might be able to
>> eliminate this one in clone. setup_reference() adds the reference
>> repository to the $GIT_DIR/objects/info/alternates, and the fetch logic
>> already knows to account for the refs in alternate repositories via
>> insert_alternate_refs() callchain.
>
> If I comment out the call from add_one_reference() to add_extra_ref()
> then I get a single failure, in t5700:
>
> not ok - 8 fetched no objects
> # ! grep "^want" "$U"
>
> So your suspicion does not seem to be borne out (at least not in the
> naivest form).
>
> Still studying...
I finally had some time to get back to this puzzle. It took me a while
to narrow down the problem, and I still don't understand it entirely.
It seems like Junio should be right that setup_reference() doesn't need
to add the alternate references to extra_refs. Indeed, if I remove the
call to add_extra_ref(), I see the alternate references being added to
ref_list via insert_one_alternate_ref(). However, in the context of
t5700, clone nevertheless sends "want" lines for one of those references
and test 8 fails. Something is screwy.
So how do the extra_refs prevent the "want" lines from being emitted?
It turns out that when the alternate refs *are* added as extra_refs,
then find_common() is not called at all. do_fetch_pack() calls
everything_local(), which returns true, and do_fetch_pack() skips over
the call to find_common().
So ISTM that there are two problems:
First problem: everything_local() seems to be either broken or used
incorrectly. I can't decide which because I don't know what its
semantics are *supposed* to be.
If everything_local() is trying to check that the references are all in
the local repository itself, then it is incorrect for clone to enter
alternates into extra_refs because everything_local() then mistakes them
for local.
If everything_local() is trying to check that the references are in the
local repository plus alternates, then it is incorrect that
everything_local() doesn't consider alternate references in its
determination. My guess is that this is the case, and that something
like the following might be the fix:
> ----------------------------- builtin/fetch-pack.c -----------------------------
> index 9500f35..4257a8d 100644
> @@ -581,6 +581,11 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
> *refs = newlist;
> }
>
> +static void mark_alternate_complete(const struct ref *ref, void *unused)
> +{
> + mark_complete(NULL, ref->old_sha1, 0, NULL);
> +}
> +
> static int everything_local(struct ref **refs, int nr_match, char **match)
> {
> struct ref *ref;
> @@ -609,6 +614,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
>
> if (!args.depth) {
> for_each_ref(mark_complete, NULL);
> + for_each_alternate_ref(mark_alternate_complete, NULL);
> if (cutoff)
> mark_recent_complete_commits(cutoff);
> }
With this patch, then the full test suite passes even if I take out the
code that adds the alternate refs to extra_refs.
Second problem: it seems like the presence of alternate refs is not
suppressing the "want" lines for those refs in all cases. Strangely, in
the case of t5700, adding the two alternate refs to ref_list
insert_one_alternate_ref() causes the "want" line for one of them to be
suppressed, but not for the other.
Specifically: (without the above patch) I commented out the call to
add_extra_ref() in clone.c:add_one_reference(), then ran t5700 through
step 8 then aborted. insert_one_alternate_ref() was called four times:
insert_one_alternate_ref(ccc25a1f9655742174c93f48f616bea8ad0bc6ff)
insert_one_alternate_ref(ccc25a1f9655742174c93f48f616bea8ad0bc6ff)
insert_one_alternate_ref(5355551c5a927a2b6349505ada2da4bb702c0a49)
insert_one_alternate_ref(5355551c5a927a2b6349505ada2da4bb702c0a49)
(The duplication here seems strange.) However, UPLOAD_LOG still
contains "want" lines (2 of them!) for one of the commits:
#S
#E
#S
#S
#E
want 5355551c5a927a2b6349505ada2da4bb702c0a49 multi_ack_detailed
side-band-64k thin-pack ofs-delta
want 5355551c5a927a2b6349505ada2da4bb702c0a49
#E
The "5355551c" object corresponds to refs/remotes/origin/master in the
alternate object store:
$ (cd B; git for-each-ref)
ccc25a1f9655742174c93f48f616bea8ad0bc6ff commit refs/heads/master
5355551c5a927a2b6349505ada2da4bb702c0a49 commit refs/remotes/origin/HEAD
5355551c5a927a2b6349505ada2da4bb702c0a49 commit refs/remotes/origin/master
It seems to me that even in the absence of short-circuiting due to
everything_local() returning true, the presence of the alternate refs
should be suppressing the "want" lines for those references.
I have some patches that seem to work (and get rid of extra_refs
entirely, hurrah!), but I don't want to submit them until I understand
what the correct behavior is *supposed* to be.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Nguyen Thai Ngoc Duy @ 2012-02-10 14:37 UTC (permalink / raw)
To: Piotr Krukowiecki; +Cc: Git Mailing List
In-Reply-To: <CAA01Csq+zVhRTjGqy1DJGK7o5wByd9ADsSim214T7Vkxmk+ykQ@mail.gmail.com>
On Fri, Feb 10, 2012 at 8:46 PM, Piotr Krukowiecki
<piotr.krukowiecki@gmail.com> wrote:
> On Fri, Feb 10, 2012 at 1:33 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>> On Fri, Feb 10, 2012 at 4:42 PM, Piotr Krukowiecki
>> <piotr.krukowiecki@gmail.com> wrote:
>>> Hi,
>>>
>>> I compared stating whole tree vs one small subdirectory, and I
>>> expected that for the subdirectory status will be very very fast.
>>> After all, it has only few files to stat. But it's not fast. Why?
>>
>> Because stat'ing is not the only thing git-status does? In order to
>> find out staged changes, unstaged changes and untracked files, it has
>> to do the equivalence of "git diff --cached", "git diff" and "git
>> ls-files -o". I think copy detection is also enabled, which uses more
>> cycles.
>
> I believe copy detection is not done, neither for tracked nor untracked files.
> Rename detection is done for tracked files. In this case it should not
> matter, as there were no changes to the files.
>
> My point is, that for such small number of small files (55 files and
> 223KB), which had no changes at all, the status took a lot of time (17
> seconds) and doing status on whole repository which has more than
> 2000x files and 10000x data took only 2x more time.
>
> I don't think that the algorithm scales so well, so my guess is that
> 'status' is so inefficient for subdirectories (i.e. the "git status --
> dir" filter does not filter as much as it could).
I think the cost is in $GIT_DIR, not the working directory.
--
Duy
^ permalink raw reply
* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Michał Kiedrowicz @ 2012-02-10 14:15 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m339aivn4z.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> wrote:
> Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:
>
> > The code that comares lines is based on
> > contrib/diff-highlight/diff-highlight, except that it works with
> > multiline changes too. It also won't highlight lines that are
> > completely different because that would only make the output unreadable.
> > Combined diffs are not supported but a following commit will change it.
>
> I was thinking that if gitweb were to support "diff refinement
> highlighting", it would either use one of *Diff* packages from CPAN,
> or "git diff --word-diff" output.
>
I think highlighting inline and side-by-side diff outputs is
something different from "git diff --word-diff". I find it useful for
people who are used to these diff formats (i.e. me :).
OTOH I'm not against using a dedicated package from CPAN. But I think
my approach is proven to work (I use contrib/diff-highlight as a
filter) and more lightweight (doesn't add another dependency to
gitweb). Moreover, adding support for some Diff package may be done
later, at any moment. It's just a matter of replacing one function
(format_rem_add_line()) with the one that uses Diff.
^ permalink raw reply
* Re: [PATCH 1/5] gitweb: Option for filling only specified info in fill_project_list_info
From: Jakub Narebski @ 2012-02-10 13:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcnftvb5.fsf@alter.siamese.dyndns.org>
On Fri, 10 Feb 2012, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> On Fri, 10 Feb 2012, Junio C Hamano wrote:
>> ...
>>> That still does not answer the fundamental issues I had with the presented
>>> API: why does it take only a single $key (please re-read my "A, B and C"
>>> example), and what does that single $key intersecting with @fill_only have
>>> anything to do with "needs-filling"?
>>
>> project_info_needs_filling() in absence of @fill_only is just a thin
>> wrapper around "!defined $pr->{$key}", it checks for each key if it needs
>> to be filled.
>>
>> It is used like this
>>
>> if (project_info_needs_filled("A", "A, B, C")) {
>> fill A
>> }
>> if (project_info_needs_filled("B", "A, B, C")) {
>> fill B
>> }
>> ...
>>
>>> After all, that 'age' check actually wants to fill 'age' and 'age_string'
>>> in the project. [...]
Thanks for noticing that... though I think more important is that some
further command would mark only 'age_string' as needed, and
fill_project_list_info() wouldn't notice that it needs to run
git_get_last_activity() if it checks only 'age'.
>> It is not as much matter of API, as the use of checks in loop in
>> fill_project_list_info().
>>
>> What is now
>>
>> my (@activity) = git_get_last_activity($pr->{'path'});
>> unless (@activity) {
>> next PROJECT;
>> }
>> ($pr->{'age'}, $pr->{'age_string'}) = @activity;
>>
>> should be
>>
>> if (!defined $pr->{'age'} ||
>> !defined $pr->{'age_string'}) {
>> my (@activity) = git_get_last_activity($pr->{'path'});
>> unless (@activity) {
>> next PROJECT;
>> }
>> ($pr->{'age'}, $pr->{'age_string'}) = @activity;
>> }
>
> Huh? Compare that with what you wrote above "It is used like this". This
> is *NOT* using the API like that. The caller knows it wants both age and
> age-string, and if even one of them is missing, do the work to fill both.
>
> So why isn't the info-needs-filled API not checking _both_ with a single
> call? It is only because you designed the API to accept only a single $key
> instead of list of "here are what I care about".
Passing two lists as parameters is a bit harder and makes for more
complicated API.
So what you mean is that instead of proposed
if (project_info_needs_filled($pr, 'age', @fill_only) ||
project_info_needs_filled($pr, 'age_string', @fill_only) {
it should be
if (project_info_needs_filled($pr, 'age', 'age_string', \@fill_only) {
or
if (project_info_needs_filled($pr, ['age', 'age_string'], @fill_only) {
(with either "..., 'owner', ..." or "..., [ 'owner' ], ..." for single-key
filling), or
if (project_info_needs_filled($pr, ['age', 'age_string'], \@fill_only) {
Is it?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Piotr Krukowiecki @ 2012-02-10 13:46 UTC (permalink / raw)
To: Git Mailing List; +Cc: Nguyen Thai Ngoc Duy
In-Reply-To: <CACsJy8DnqYZ5CdZqbebWS4NS85mfwumyao0abeqDxMXrHhDELA@mail.gmail.com>
On Fri, Feb 10, 2012 at 1:33 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Fri, Feb 10, 2012 at 4:42 PM, Piotr Krukowiecki
> <piotr.krukowiecki@gmail.com> wrote:
>> Hi,
>>
>> I compared stating whole tree vs one small subdirectory, and I
>> expected that for the subdirectory status will be very very fast.
>> After all, it has only few files to stat. But it's not fast. Why?
>
> Because stat'ing is not the only thing git-status does? In order to
> find out staged changes, unstaged changes and untracked files, it has
> to do the equivalence of "git diff --cached", "git diff" and "git
> ls-files -o". I think copy detection is also enabled, which uses more
> cycles.
I believe copy detection is not done, neither for tracked nor untracked files.
Rename detection is done for tracked files. In this case it should not
matter, as there were no changes to the files.
My point is, that for such small number of small files (55 files and
223KB), which had no changes at all, the status took a lot of time (17
seconds) and doing status on whole repository which has more than
2000x files and 10000x data took only 2x more time.
I don't think that the algorithm scales so well, so my guess is that
'status' is so inefficient for subdirectories (i.e. the "git status --
dir" filter does not filter as much as it could).
> Profiling it should give you a good idea what parts cost most.
I'll try that later.
BTW by stating I meant using "status", not that it uses stat() or whatever.
--
Piotr Krukowiecki
^ permalink raw reply
* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Jakub Narebski @ 2012-02-10 13:23 UTC (permalink / raw)
To: Michał Kiedrowicz; +Cc: git
In-Reply-To: <1328865494-24415-7-git-send-email-michal.kiedrowicz@gmail.com>
Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:
> The code that comares lines is based on
> contrib/diff-highlight/diff-highlight, except that it works with
> multiline changes too. It also won't highlight lines that are
> completely different because that would only make the output unreadable.
> Combined diffs are not supported but a following commit will change it.
I was thinking that if gitweb were to support "diff refinement
highlighting", it would either use one of *Diff* packages from CPAN,
or "git diff --word-diff" output.
--
Jakub Narębski
^ permalink raw reply
* Re: 1.7.9, libcharset missing from EXTLIBS
From: Jakub Narebski @ 2012-02-10 13:15 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Junio C Hamano,
Дилян Палаузов,
git
In-Reply-To: <CACBZZX45=mr=FMqFF+Pw4KPaDAtvs-ePLbFATpyFA93vSfZatw@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 2012/2/10 Junio C Hamano <gitster@pobox.com>:
>> Дилян Палаузов <dilyan.palauzov@aegee.org> writes:
>>> git 1.7.9 makes use of libcharset and /Makefile contains:
> >>
>>> ifdef HAVE_LIBCHARSET_H
>>> BASIC_CFLAGS += -DHAVE_LIBCHARSET_H
>>> endif
>>> ...
>>> and the problem is, that libcharset is not used when linking. To
>>> solve this, please replace the above extract from /Makefile with
>>>
>>> ifdef HAVE_LIBCHARSET_H
>>> BASIC_CFLAGS += -DHAVE_LIBCHARSET_H
>>> EXTLIBS += -lcharset
>>> endif
[....]
> I've had some similar (privately sent) bug reports about the i18n
> stuff from someone who built his own Linux distro.
>
> Basically we make assumptions that certain stuff will be in the C
> library on certain platforms, certain headers go with certain
> libraries etc.
>
> Evidently none of this can really be relied upon and we'd have to
> probe for each one if we wanted to make it reliable.
Well, there is always *optional* configure.ac to do detection;
"make configure; ./configure <options>" will generate config.mak.autogen
with appropriate options -- we can add autodetection if -lcharset
must be added.
--
Jakub Narębski
^ permalink raw reply
* Bulgarian translation of git
From: Alexander Shopov (Александър Шопов) @ 2012-02-10 12:26 UTC (permalink / raw)
To: git
Hello,
I am Alexander Shopov and would like to volunteer for the Bulgarian
translation of git po file if no one else is doing this.
How should I contribute it back?
Via this email list, via fork-push from github, or a patch?
Kind regards:
al_shopov
^ permalink raw reply
* Re: [PATCH 3/4] diff --stat: use the real terminal width
From: Nguyen Thai Ngoc Duy @ 2012-02-10 13:00 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, gitster, Michael J Gruber
In-Reply-To: <4F34FE9A.7020600@in.waw.pl>
2012/2/10 Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>:
>> I tried this in the past and "git log -p" looked ugly on git.git
>> mainly because commit messages are still ~70 char long lines on my 279
>> char wide terminal. If this is project dependent, perhaps a config
>> key? Also the "50" below the changed line, maybe you want to change it
>> to 0.6 * width.
>
>
> Thanks for all the comments. I'll post a newer version, but I have two
> questions:
>
> I agree that making the output very wide with lots of +- is not very
> elegant. (E.g. 8f24a6323ece9be1bf1a04b4b5856112438337f2 has
> builtin/grep.c | 142 +++--------------------------------....--
> which doesn't look right.). So I think it would make sense to limit
> the graph part to something like 50 columns, even if there's more space.
> I believe that git.git would look fine with this change. There are some
> fairly long lines
> (t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
> is 86 chars) but with 50 columns of graph the output would take 140 columns
> -- with the graph part slightly sticking out from the 80 column
> descriptions, but still not too ugly.
You also need to pay attention, not to exceed term_columns() because
of this dynamic name part. But the idea sounds good to me.
It would be even better if we had some heuristics to shorten
exceptionally long names with ellipsis, to keep the name part from
being overly stretched.
> Should I add a new option --stat-graph-width in analogy to
> --stat-name-width, or should this be hard-coded?
I don't know about other people, but I have no urge to change graph width.
--
Duy
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Nguyen Thai Ngoc Duy @ 2012-02-10 12:33 UTC (permalink / raw)
To: Piotr Krukowiecki; +Cc: Git Mailing List
In-Reply-To: <CAA01Csr8FbvQ8uFvxX8_6i-hysin6JuaifVVC-yoLyoT0N5F4Q@mail.gmail.com>
On Fri, Feb 10, 2012 at 4:42 PM, Piotr Krukowiecki
<piotr.krukowiecki@gmail.com> wrote:
> Hi,
>
> I compared stating whole tree vs one small subdirectory, and I
> expected that for the subdirectory status will be very very fast.
> After all, it has only few files to stat. But it's not fast. Why?
Because stat'ing is not the only thing git-status does? In order to
find out staged changes, unstaged changes and untracked files, it has
to do the equivalence of "git diff --cached", "git diff" and "git
ls-files -o". I think copy detection is also enabled, which uses more
cycles.
Profiling it should give you a good idea what parts cost most.
--
Duy
^ permalink raw reply
* Re: Git performance results on a large repository
From: Nguyen Thai Ngoc Duy @ 2012-02-10 12:24 UTC (permalink / raw)
To: Christian Couder; +Cc: Joshua Redstone, git@vger.kernel.org
In-Reply-To: <CAP8UFD1RTa6+btjJrsfqjOOoCjebZBqK6xkPN7ZVLM04bHO9yw@mail.gmail.com>
On Fri, Feb 10, 2012 at 4:39 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> Hi,
>
> On Fri, Feb 10, 2012 at 8:12 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>>
>> All these involve unpack_trees(), which is full tree operation. The
>> bigger your worktree is, the slower it is. Another good reason to
>> split unrelated parts into separate repositories.
>
> Maybe having different "views" would be enough to make a smaller
> worktree and history, so that things are much faster for a developper?
>
> (I already suggested "views" based on "git replace" in this thread:
> http://thread.gmane.org/gmane.comp.version-control.git/177146/focus=177639)
That's more or less what I did with the subtree clone series [1] and
ended up doing narrow clone [2]. The only difference between the two
are how to handle partial worktree/index. The former uses git-replace
to seal any holes, the latter tackles at pathspec level and is
generally more elegant.
The worktree part from that work should be usable in full clone too. I
am reviving the series and going to repost it soon. Have a look [3] if
you are interested.
[1] http://thread.gmane.org/gmane.comp.version-control.git/152347
[2] http://thread.gmane.org/gmane.comp.version-control.git/155427
[3] https://github.com/pclouds/git/commits/narrow-clone
--
Duy
^ permalink raw reply
* Re: A note on modern git plus ancient meld ("wrong number of arguments")
From: Sebastian Schuberth @ 2012-02-10 11:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: David Aguilar, Jeff Epler, git, Charles Bailey
In-Reply-To: <20120210082106.GA7871@burratino>
On Fri, Feb 10, 2012 at 09:23, Jonathan Nieder <jrnieder@gmail.com> wrote:
> + meld_version=${meld_version#GNOME }
> + meld_version=${meld_version#* }
Hmm, I might be mistaken, but aren't these string operations
Bash-only? And AFAIK Git is striving for standard sh compatibility ...
--
Sebastian Schuberth
^ permalink raw reply
* Re: [PATCH 3/4] diff --stat: use the real terminal width
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-10 11:25 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy, git, gitster; +Cc: Michael J Gruber
In-Reply-To: <CACsJy8APGeTNv_E3qD=xFCiLC25M_nm3aJbq6YU73J=X0Wxh2w@mail.gmail.com>
On 02/10/2012 07:15 AM, Nguyen Thai Ngoc Duy wrote:
> 2012/2/10 Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>:
>> @@ -1341,7 +1342,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>> line_prefix = msg->buf;
>> }
>>
>> - width = options->stat_width ? options->stat_width : 80;
>> + width = options->stat_width ? options->stat_width : term_columns();
>> name_width = options->stat_name_width ? options->stat_name_width : 50;
>> count = options->stat_count ? options->stat_count : data->nr;
>
> I tried this in the past and "git log -p" looked ugly on git.git
> mainly because commit messages are still ~70 char long lines on my 279
> char wide terminal. If this is project dependent, perhaps a config
> key? Also the "50" below the changed line, maybe you want to change it
> to 0.6 * width.
Thanks for all the comments. I'll post a newer version, but I have two
questions:
I agree that making the output very wide with lots of +- is not very
elegant. (E.g. 8f24a6323ece9be1bf1a04b4b5856112438337f2 has
builtin/grep.c | 142 +++--------------------------------....--
which doesn't look right.). So I think it would make sense to limit
the graph part to something like 50 columns, even if there's more space.
I believe that git.git would look fine with this change. There are some
fairly long lines
(t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
is 86 chars) but with 50 columns of graph the output would take 140
columns -- with the graph part slightly sticking out from the 80 column
descriptions, but still not too ugly.
Should I add a new option --stat-graph-width in analogy to
--stat-name-width, or should this be hard-coded?
JC:
> The output from "git format-patch" shouldn't be affected at all by the
> width of the terminal the patch sender happened to have used when the
> command was run when the user did not explicitly ask a custom width by
> giving a --stat-width command line option.
>
> How do you prevent regression to the command in this series?
git format-patch is not affected by default. But with --stdout
the width is changed, iff stdout is a tty. When --stdout output
is connected to a pipe, the width is not changed. I think that
this behaviour is OK.
Should a test be added to check that 'git format-patch --stat' output
doesn't change? Should I test for something else?
--
Zbyszek
^ permalink raw reply
* Re: 1.7.9, libcharset missing from EXTLIBS
From: Дилян Палаузов @ 2012-02-10 10:21 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git
In-Reply-To: <CACBZZX45=mr=FMqFF+Pw4KPaDAtvs-ePLbFATpyFA93vSfZatw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]
Hello,
>> What platform is this? Is there a guarantee that any and all system that
>> use "#include<libcharset.h>" has to link with "-lcharset"?
>
> I've had some similar (privately sent) bug reports about the i18n
> stuff from someone who built his own Linux distro.
I run Linux from scratch.
>> What I am wondering is there are systems that need to include the header,
>> but locale_charset() does not live in /lib/libcharset.a, in which case we
>> cannot make HAVE_LIBCHARSET_H imply use of -lcharset.
I do not understand this. If you want to use a function from
libcharset, you have to use both #include <libcharset.h> and -lcharset.
Със здраве
Дилян
On 10.02.2012 11:06, Ævar Arnfjörð Bjarmason wrote:
> 2012/2/10 Junio C Hamano<gitster@pobox.com>:
>> Дилян Палаузов<dilyan.palauzov@aegee.org> writes:
>>
>>> Hello,
>>>
>>> git 1.7.9 makes use of libcharset and /Makefile contains:
>>>
>>> ifdef HAVE_LIBCHARSET_H
>>> BASIC_CFLAGS += -DHAVE_LIBCHARSET_H
>>> endif
>>> ...
>>> and the problem is, that libcharset is not used when linking. To
>>> solve this, please replace the above extract from /Makefile with
>>>
>>> ifdef HAVE_LIBCHARSET_H
>>> BASIC_CFLAGS += -DHAVE_LIBCHARSET_H
>>> EXTLIBS += -lcharset
>>> endif
>>
>> Thanks.
>>
>> What platform is this? Is there a guarantee that any and all system that
>> use "#include<libcharset.h>" has to link with "-lcharset"?
>>
>> What I am wondering is there are systems that need to include the header,
>> but locale_charset() does not live in /lib/libcharset.a, in which case we
>> cannot make HAVE_LIBCHARSET_H imply use of -lcharset.
>
> I've had some similar (privately sent) bug reports about the i18n
> stuff from someone who built his own Linux distro.
>
> Basically we make assumptions that certain stuff will be in the C
> library on certain platforms, certain headers go with certain
> libraries etc.
>
> Evidently none of this can really be relied upon and we'd have to
> probe for each one if we wanted to make it reliable.
[-- Attachment #2: dilyan_palauzov.vcf --]
[-- Type: text/x-vcard, Size: 381 bytes --]
begin:vcard
fn;quoted-printable:=D0=94=D0=B8=D0=BB=D1=8F=D0=BD =D0=9F=D0=B0=D0=BB=D0=B0=D1=83=D0=B7=D0=BE=
=D0=B2
n;quoted-printable;quoted-printable:=D0=9F=D0=B0=D0=BB=D0=B0=D1=83=D0=B7=D0=BE=D0=B2;=D0=94=D0=B8=D0=BB=D1=8F=D0=BD
email;internet:dilyan.palauzov@aegee.org
tel;home:+49-721-94193270
tel;cell:+49-162-4091172
note:sip:8372@aegee.org
version:2.1
end:vcard
^ permalink raw reply
* Re: 1.7.9, libcharset missing from EXTLIBS
From: Ævar Arnfjörð Bjarmason @ 2012-02-10 10:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: Дилян Палаузов,
git
In-Reply-To: <7v1uq3toyq.fsf@alter.siamese.dyndns.org>
2012/2/10 Junio C Hamano <gitster@pobox.com>:
> Дилян Палаузов <dilyan.palauzov@aegee.org> writes:
>
>> Hello,
>>
>> git 1.7.9 makes use of libcharset and /Makefile contains:
>>
>> ifdef HAVE_LIBCHARSET_H
>> BASIC_CFLAGS += -DHAVE_LIBCHARSET_H
>> endif
>> ...
>> and the problem is, that libcharset is not used when linking. To
>> solve this, please replace the above extract from /Makefile with
>>
>> ifdef HAVE_LIBCHARSET_H
>> BASIC_CFLAGS += -DHAVE_LIBCHARSET_H
>> EXTLIBS += -lcharset
>> endif
>
> Thanks.
>
> What platform is this? Is there a guarantee that any and all system that
> use "#include <libcharset.h>" has to link with "-lcharset"?
>
> What I am wondering is there are systems that need to include the header,
> but locale_charset() does not live in /lib/libcharset.a, in which case we
> cannot make HAVE_LIBCHARSET_H imply use of -lcharset.
I've had some similar (privately sent) bug reports about the i18n
stuff from someone who built his own Linux distro.
Basically we make assumptions that certain stuff will be in the C
library on certain platforms, certain headers go with certain
libraries etc.
Evidently none of this can really be relied upon and we'd have to
probe for each one if we wanted to make it reliable.
^ permalink raw reply
* git status: small difference between stating whole repository and small subdirectory
From: Piotr Krukowiecki @ 2012-02-10 9:42 UTC (permalink / raw)
To: Git Mailing List
Hi,
I compared stating whole tree vs one small subdirectory, and I
expected that for the subdirectory status will be very very fast.
After all, it has only few files to stat. But it's not fast. Why?
With cold cache (echo 3 | sudo tee /proc/sys/vm/drop_caches):
$ time git status > /dev/null
real 0m41.670s
user 0m0.980s
sys 0m2.908s
$ time git status -- src/.../somedir > /dev/null
real 0m17.380s
user 0m0.748s
sys 0m0.328s
With warm cache:
$ time git status > /dev/null
real 0m0.792s
user 0m0.404s
sys 0m0.384s
$ time git status -- src/.../somedir > /dev/null
real 0m0.335s
user 0m0.288s
sys 0m0.048s
Repository/dir stats:
$ find * -type f | wc -l
127189
$ du -shc * | grep total
2.2G total
$ find src/.../somedir -type f | wc -l
55
$ du -shc src/.../somedir | grep total
224K total
$ git --version
git version 1.7.9.rc0.10.gbeecc
--
Piotr Krukowiecki
^ permalink raw reply
* Re: Git performance results on a large repository
From: Christian Couder @ 2012-02-10 9:39 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Joshua Redstone, git@vger.kernel.org
In-Reply-To: <CACsJy8DQNHm8sTgxKL=+Ui5OBsJBpvPn+dRmN9bVMwq4TfNuxQ@mail.gmail.com>
Hi,
On Fri, Feb 10, 2012 at 8:12 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>
> All these involve unpack_trees(), which is full tree operation. The
> bigger your worktree is, the slower it is. Another good reason to
> split unrelated parts into separate repositories.
Maybe having different "views" would be enough to make a smaller
worktree and history, so that things are much faster for a developper?
(I already suggested "views" based on "git replace" in this thread:
http://thread.gmane.org/gmane.comp.version-control.git/177146/focus=177639)
Best regards,
Christian.
^ permalink raw reply
* [PATCH 1/8] gitweb: Extract print_sidebyside_diff_lines()
From: Michał Kiedrowicz @ 2012-02-10 9:18 UTC (permalink / raw)
To: git; +Cc: Michał Kiedrowicz
In-Reply-To: <1328865494-24415-1-git-send-email-michal.kiedrowicz@gmail.com>
Currently, print_sidebyside_diff_chunk() does two things: it
accumulates diff lines and prints them. Accumulation may be used to
perform additional operations on diff lines, so it makes sense to split
these two things. Thus, the code that prints diff lines in a side-by-side
manner is moved out of print_sidebyside_diff_chunk() to a separate
subroutine.
The outcome of this patch is that print_sidebyside_diff_chunk() is now
much shorter and easier to read.
This change is meant to be a simple code movement. No behavior change is
intended.
Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---
gitweb/gitweb.perl | 97 ++++++++++++++++++++++++++++------------------------
1 files changed, 52 insertions(+), 45 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..1247607 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4861,6 +4861,53 @@ sub git_difftree_body {
print "</table>\n";
}
+# Print context lines and then rem/add lines in a side-by-side manner.
+sub print_sidebyside_diff_lines {
+ my ($ctx, $rem, $add) = @_;
+
+ # print context block before add/rem block
+ if (@$ctx) {
+ print join '',
+ '<div class="chunk_block ctx">',
+ '<div class="old">',
+ @$ctx,
+ '</div>',
+ '<div class="new">',
+ @$ctx,
+ '</div>',
+ '</div>';
+ }
+
+ if (!@$add) {
+ # pure removal
+ print join '',
+ '<div class="chunk_block rem">',
+ '<div class="old">',
+ @$rem,
+ '</div>',
+ '</div>';
+ } elsif (!@$rem) {
+ # pure addition
+ print join '',
+ '<div class="chunk_block add">',
+ '<div class="new">',
+ @$add,
+ '</div>',
+ '</div>';
+ } else {
+ # assume that it is change
+ print join '',
+ '<div class="chunk_block chg">',
+ '<div class="old">',
+ @$rem,
+ '</div>',
+ '<div class="new">',
+ @$add,
+ '</div>',
+ '</div>';
+ }
+}
+
sub print_sidebyside_diff_chunk {
my @chunk = @_;
my (@ctx, @rem, @add);
@@ -4887,51 +4934,11 @@ sub print_sidebyside_diff_chunk {
next;
}
- ## print from accumulator when type of class of lines change
- # empty contents block on start rem/add block, or end of chunk
- if (@ctx && (!$class || $class eq 'rem' || $class eq 'add')) {
- print join '',
- '<div class="chunk_block ctx">',
- '<div class="old">',
- @ctx,
- '</div>',
- '<div class="new">',
- @ctx,
- '</div>',
- '</div>';
- @ctx = ();
- }
- # empty add/rem block on start context block, or end of chunk
- if ((@rem || @add) && (!$class || $class eq 'ctx')) {
- if (!@add) {
- # pure removal
- print join '',
- '<div class="chunk_block rem">',
- '<div class="old">',
- @rem,
- '</div>',
- '</div>';
- } elsif (!@rem) {
- # pure addition
- print join '',
- '<div class="chunk_block add">',
- '<div class="new">',
- @add,
- '</div>',
- '</div>';
- } else {
- # assume that it is change
- print join '',
- '<div class="chunk_block chg">',
- '<div class="old">',
- @rem,
- '</div>',
- '<div class="new">',
- @add,
- '</div>',
- '</div>';
- }
- @rem = @add = ();
+ ## print from accumulator when have some add/rem lines or end
+ # of chunk (flush context lines)
+ if (((@rem || @add) && $class eq 'ctx') || !$class) {
+ print_sidebyside_diff_lines(\@ctx, \@rem, \@add);
+ @ctx = @rem = @add = ();
}
## adding lines to accumulator
--
1.7.3.4
^ permalink raw reply related
* [PATCH 0/8] gitweb: Highlight interesting parts of diff
From: Michał Kiedrowicz @ 2012-02-10 9:18 UTC (permalink / raw)
To: git; +Cc: Michał Kiedrowicz
Reading diff output is sometimes very hard, even if it's colored,
especially if lines differ only in few characters. This is often true when
a commit fixes a typo or renames some variables or functions.
This patch series teaches gitweb to highlight fragments that are different
between old and new line. This should mimic the same feature in Trac or
GitHub.
To achieve that, added and removed lines must be accumulated and compared.
The current code in print_sidebyside_diff_chunk() already does this, so
this patch series reuses it in commits:
gitweb: Extract print_sidebyside_diff_lines()
gitweb: Use print_diff_chunk() for both side-by-side and inline diffs
Next, HTML-formatting diff lines is pushed down to the place where they
are about to be printed. This is required because comparision must be
performened on raw git-diff output and not on HTML-formatted lines. This
is done in commits:
gitweb: Move HTML-formatting diff line back to process_diff_line()
gitweb: Push formatting diff lines to print_diff_chunk()
gitweb: Format diff lines just before printing
The last three commits implement the advertised feature. They could be
squashed together but that would make them harder to review (I think).
gitweb: Highlight interesting parts of diff
gitweb: Use different colors to present marked changes
gitweb: Highlight combined diffs
This series is based on v1.7.9.
Michał Kiedrowicz (8):
gitweb: Extract print_sidebyside_diff_lines()
gitweb: Use print_diff_chunk() for both side-by-side and inline diffs
gitweb: Move HTML-formatting diff line back to process_diff_line()
gitweb: Push formatting diff lines to print_diff_chunk()
gitweb: Format diff lines just before printing
gitweb: Highlight interesting parts of diff
gitweb: Use different colors to present marked changes
gitweb: Highlight combined diffs
gitweb/gitweb.perl | 304 +++++++++++++++++++++++++++++++++++-----------
gitweb/static/gitweb.css | 8 ++
2 files changed, 238 insertions(+), 74 deletions(-)
--
1.7.3.4
^ permalink raw reply
* [PATCH 8/8] gitweb: Highlight combined diffs
From: Michał Kiedrowicz @ 2012-02-10 9:18 UTC (permalink / raw)
To: git; +Cc: Michał Kiedrowicz
In-Reply-To: <1328865494-24415-1-git-send-email-michal.kiedrowicz@gmail.com>
The highlightning of combined diffs is currently disabled. This is
because output from a combined diff is much harder to highlight because
it's not obvious which removed and added lines should be compared.
Moreover, code that compares added and removed lines doesn't care about
combined diffs. It only skips first +/- character, treating second +/-
as a line content.
Let's start with a simple case: only highlight changes that come from
one parent, i.e. when every removed line has a corresponding added line
for the same parent. This way the highlightning cannot get wrong. For
example, following diffs would be highlighted:
- removed line for first parent
+ added line for first parent
context line
-removed line for second parent
+added line for second parent
or
- removed line for first parent
-removed line for second parent
+ added line for first parent
+added line for second parent
but following output will not:
- removed line for first parent
-removed line for second parent
+added line for second parent
++added line for both parents
Further changes may introduce more intelligent approach that better
handles combined diffs.
Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---
gitweb/gitweb.perl | 40 +++++++++++++++++++++++++++++++++++++---
1 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1a5b454..2b6cb9e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4944,13 +4944,16 @@ sub esc_html_mark_range {
# Format removed and added line, mark changed part and HTML-format them.
sub format_rem_add_line {
- my ($rem, $add) = @_;
+ my ($rem, $add, $is_combined) = @_;
my @r = split(//, $rem);
my @a = split(//, $add);
my ($esc_rem, $esc_add);
my ($prefix, $suffix_rem, $suffix_add) = (1, $#r, $#a);
my ($prefix_is_space, $suffix_is_space) = (1, 1);
+ # In combined diff we must ignore two +/- characters.
+ $prefix = 2 if ($is_combined);
+
while ($prefix < @r && $prefix < @a) {
last if ($r[$prefix] ne $a[$prefix]);
@@ -4988,11 +4991,42 @@ sub format_ctx_rem_add_lines {
my ($ctx, $rem, $add, $is_combined) = @_;
my (@new_ctx, @new_rem, @new_add);
my $num_add_lines = @$add;
+ my $can_highlight;
+
+ # Highlight if every removed line has a corresponding added line.
+ if ($num_add_lines > 0 && $num_add_lines == @$rem) {
+ $can_highlight = 1;
+
+ # Highlight lines in combined diff only if the chunk contains
+ # diff between the same version, e.g.
+ #
+ # - a
+ # - b
+ # + c
+ # + d
+ #
+ # Otherwise the highlightling would be confusing.
+ if ($is_combined) {
+ for (my $i = 0; $i < $num_add_lines; $i++) {
+ my $prefix_rem = substr($rem->[$i], 0, 2);
+ my $prefix_add = substr($add->[$i], 0, 2);
+
+ $prefix_rem =~ s/-/+/g;
+
+ if ($prefix_rem ne $prefix_add) {
+ $can_highlight = 0;
+ last;
+ }
+ }
+ }
+ } else {
+ $can_highlight = 0;
+ }
- if (!$is_combined && $num_add_lines > 0 && $num_add_lines == @$rem) {
+ if ($can_highlight) {
for (my $i = 0; $i < $num_add_lines; $i++) {
my ($line_rem, $line_add) = format_rem_add_line(
- $rem->[$i], $add->[$i]);
+ $rem->[$i], $add->[$i], $is_combined);
push @new_rem, $line_rem;
push @new_add, $line_add;
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH 7/8] gitweb: Use different colors to present marked changes
From: Michał Kiedrowicz @ 2012-02-10 9:18 UTC (permalink / raw)
To: git; +Cc: Michał Kiedrowicz
In-Reply-To: <1328865494-24415-1-git-send-email-michal.kiedrowicz@gmail.com>
This makes use of the highlight diff feature.
Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---
I decided to split mechanism (generate HTML page with <span> elements that
mark interesting fragments of diff output) from politics (use these
particular colors for this <span> elements), but otherwise this commit
may be squashed with the previous one. These colors work for me but if
someone comes out with better ones, I'd be happy.
gitweb/static/gitweb.css | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index c7827e8..4f87d16 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -438,6 +438,10 @@ div.diff.add {
color: #008800;
}
+div.diff.add span.marked {
+ background-color: #77ff77;
+}
+
div.diff.from_file a.path,
div.diff.from_file {
color: #aa0000;
@@ -447,6 +451,10 @@ div.diff.rem {
color: #cc0000;
}
+div.diff.rem span.marked {
+ background-color: #ff7777;
+}
+
div.diff.chunk_header a,
div.diff.chunk_header {
color: #990099;
--
1.7.3.4
^ permalink raw reply related
* [PATCH 2/8] gitweb: Use print_diff_chunk() for both side-by-side and inline diffs
From: Michał Kiedrowicz @ 2012-02-10 9:18 UTC (permalink / raw)
To: git; +Cc: Michał Kiedrowicz
In-Reply-To: <1328865494-24415-1-git-send-email-michal.kiedrowicz@gmail.com>
This renames print_sidebyside_diff_chunk() to print_diff_chunk() and
makes use of it for both side-by-side and inline diffs. Now diff lines
are always accumulated before they are printed. This opens the
possibility to preprocess diff output before it's printed.
The new function print_inline_diff_lines() could reorder diff lines. It
first prints all context lines, then all removed lines and finally all
added lines. If the diff output consisted of mixed added and removed
lines, gitweb would reorder these lines. This is especially true for
combined diff output, for example:
- removed line for first parent
+ added line for first parent
-removed line for second parent
++added line for both parents
would be rendered as:
- removed line for first parent
-removed line for second parent
+ added line for first parent
++added line for both parents
To prevent gitweb from reordering lines, print_diff_chunk() calls
print_diff_lines() as soon as it detects that both added and removed
lines are present and there was a class change.
Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---
gitweb/gitweb.perl | 53 +++++++++++++++++++++++++++++++++++++--------------
1 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1247607..ed63261 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4908,9 +4908,31 @@ sub print_sidebyside_diff_lines {
}
}
-sub print_sidebyside_diff_chunk {
- my @chunk = @_;
+# Print context lines and then rem/add lines in inline manner.
+sub print_inline_diff_lines {
+ my ($ctx, $rem, $add) = @_;
+
+ print foreach (@$ctx);
+ print foreach (@$rem);
+ print foreach (@$add);
+}
+
+# Print context lines and then rem/add lines.
+sub print_diff_lines {
+ my ($ctx, $rem, $add, $diff_style, $is_combined) = @_;
+
+ if ($diff_style eq 'sidebyside' && !$is_combined) {
+ print_sidebyside_diff_lines($ctx, $rem, $add);
+ } else {
+ # default 'inline' style and unknown styles
+ print_inline_diff_lines($ctx, $rem, $add);
+ }
+}
+
+sub print_diff_chunk {
+ my ($diff_style, $is_combined, @chunk) = @_;
my (@ctx, @rem, @add);
+ my $prev_class = '';
return unless @chunk;
@@ -4935,9 +4957,13 @@ sub print_sidebyside_diff_chunk {
}
## print from accumulator when have some add/rem lines or end
- # of chunk (flush context lines)
- if (((@rem || @add) && $class eq 'ctx') || !$class) {
- print_sidebyside_diff_lines(\@ctx, \@rem, \@add);
+ # of chunk (flush context lines), or when have add and rem
+ # lines and new block is reached (otherwise add/rem lines could
+ # be reordered)
+ if (((@rem || @add) && $class eq 'ctx') || !$class ||
+ (@rem && @add && $class ne $prev_class)) {
+ print_diff_lines(\@ctx, \@rem, \@add, $diff_style,
+ $is_combined);
@ctx = @rem = @add = ();
}
@@ -4954,6 +4980,8 @@ sub print_sidebyside_diff_chunk {
if ($class eq 'ctx') {
push @ctx, $line;
}
+
+ $prev_class = $class;
}
}
@@ -5080,22 +5108,17 @@ sub git_patchset_body {
$diff_classes .= " $class" if ($class);
$line = "<div class=\"$diff_classes\">$line</div>\n";
- if ($diff_style eq 'sidebyside' && !$is_combined) {
- if ($class eq 'chunk_header') {
- print_sidebyside_diff_chunk(@chunk);
- @chunk = ( [ $class, $line ] );
- } else {
- push @chunk, [ $class, $line ];
- }
+ if ($class eq 'chunk_header') {
+ print_diff_chunk($diff_style, $is_combined, @chunk);
+ @chunk = ( [ $class, $line ] );
} else {
- # default 'inline' style and unknown styles
- print $line;
+ push @chunk, [ $class, $line ];
}
}
} continue {
if (@chunk) {
- print_sidebyside_diff_chunk(@chunk);
+ print_diff_chunk($diff_style, $is_combined, @chunk);
@chunk = ();
}
print "</div>\n"; # class="patch"
--
1.7.3.4
^ 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