* Re: Translations in Git release?
From: Johannes Gilger @ 2009-01-26 9:54 UTC (permalink / raw)
To: git
In-Reply-To: <60646ee10901250941s34f7accem1b74fc201e895a41@mail.gmail.com>
On 2009-01-25, Dill <sarpulhu@gmail.com> wrote:
> Is there a plan to include translations of the Documentation within
> Git or should they exist outside of the project?
My oppinion on localization of software (and its documentation) is
generally a negative one.
- People who use software like git are (in my experience) people who
have a solid foundation of english, especially when it comes to
computer-topics.
- The effort that goes into translating the vast git documentation and
keeping it up-to-date isn't small, energy better spent in other areas.
- Translating a lot of technical terms into a language like german is
really ugly and not fun to read. I always prefer reading english
documentation and using non-localized versions of programs as it enables
me to easier partake in discussions about it and also enables me to
google for error messages without trying every different language the
message could be in ;)
But thats just me, if you want to start a translation effort knock
yourself out
Greetings,
Jojo
--
Johannes Gilger <heipei@hackvalue.de>
http://hackvalue.de/heipei/
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81 882C 58EE B178 42F6 DE81
^ permalink raw reply
* Re: [PATCH] diff.c: output correct index lines for a split diff
From: Jeff King @ 2009-01-26 9:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pixel, git
In-Reply-To: <7vhc3m8o0b.fsf_-_@gitster.siamese.dyndns.org>
On Mon, Jan 26, 2009 at 12:33:56AM -0800, Junio C Hamano wrote:
> This moves the code to generate metainfo lines around, so that two
> independent sets of metainfo lines are used for the split halves.
The patch and the generated output look correct to me, so
Acked-by: Jeff King <peff@peff.net>
> I did not include your new test script here; perhaps we can add it to an
> existing typechange diff/apply test, like t4114?
I think it makes sense to add to t4114. Please squash in the test below.
One think to note, though: test 8 (binary -> symlink) shows breakage
with the current master, but test 9 (symlink -> binary) does not.
However, if you run the test under "-d" you can see that the diff has
bogus metainfo. It's just that "apply" doesn't care.
Your test fixes the test failure, and I verified manually that the diff
output is sensible. I don't think an additional test is worth it, but we
could add one that explicitly checks the metainfo if you want to be
extra paranoid.
---
diff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh
index 5533492..0f185ca 100755
--- a/t/t4114-apply-typechange.sh
+++ b/t/t4114-apply-typechange.sh
@@ -25,6 +25,10 @@ test_expect_success 'setup repository and commits' '
git update-index foo &&
git commit -m "foo back to file" &&
git branch foo-back-to-file &&
+ printf "\0" > foo &&
+ git update-index foo &&
+ git commit -m "foo becomes binary" &&
+ git branch foo-becomes-binary &&
rm -f foo &&
git update-index --remove foo &&
mkdir foo &&
@@ -85,6 +89,20 @@ test_expect_success 'symlink becomes file' '
'
test_debug 'cat patch'
+test_expect_success 'binary file becomes symlink' '
+ git checkout -f foo-becomes-binary &&
+ git diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&
+ git apply --index < patch
+ '
+test_debug 'cat patch'
+
+test_expect_success 'symlink becomes binary file' '
+ git checkout -f foo-symlinked-to-bar &&
+ git diff-tree -p --binary HEAD foo-becomes-binary > patch &&
+ git apply --index < patch
+ '
+test_debug 'cat patch'
+
test_expect_success 'symlink becomes directory' '
git checkout -f foo-symlinked-to-bar &&
^ permalink raw reply related
* [PATCH] rebase -i: correctly remember --root flag across --continue
From: Thomas Rast @ 2009-01-26 9:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin, Junio C Hamano
In-Reply-To: <7vtz7ma9z1.fsf@gitster.siamese.dyndns.org>
From: Junio C Hamano <gitster@pobox.com>
d911d14 (rebase -i: learn to rebase root commit, 2009-01-02) tried to
remember the --root flag across a merge conflict in a broken way.
Introduce a flag file $DOTEST/rebase-root to fix and clarify.
While at it, also make sure $UPSTREAM is always initialized to guard
against existing values in the environment.
[tr: added tests]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Junio C Hamano wrote:
> Since you never use the value stored in "$DOTEST/upstream" for anything
> else anyway, how about doing something like this instead? It would make
> the meaning of the file used as a state variable much clearer.
Yes, thanks, a patch precisely "like this" is in fact the right fix.
I came up with some tests that try a conflicted --root rebase of each
flavour, to guard against the problem in the future. I wasn't
entirely sure how to shape this into a patch, but here's a version
that forges patch message and sign-off in your name.
Dscho, with that confusion cleared, you can add my Ack to your 1/2
(unchanged, though I'm afraid you'll get a textual conflict).
git-rebase--interactive.sh | 10 ++++-
t/t3412-rebase-root.sh | 88 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+), 2 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 21ac20c..17cf0e5 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -456,7 +456,7 @@ get_saved_options () {
test -d "$REWRITTEN" && PRESERVE_MERGES=t
test -f "$DOTEST"/strategy && STRATEGY="$(cat "$DOTEST"/strategy)"
test -f "$DOTEST"/verbose && VERBOSE=t
- test ! -s "$DOTEST"/upstream && REBASE_ROOT=t
+ test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
}
while test $# != 0
@@ -585,6 +585,7 @@ first and then run 'git rebase --continue' again."
test -z "$ONTO" && ONTO=$UPSTREAM
shift
else
+ UPSTREAM=
UPSTREAM_ARG=--root
test -z "$ONTO" &&
die "You must specify --onto when using --root"
@@ -611,7 +612,12 @@ first and then run 'git rebase --continue' again."
echo "detached HEAD" > "$DOTEST"/head-name
echo $HEAD > "$DOTEST"/head
- echo $UPSTREAM > "$DOTEST"/upstream
+ case "$REBASE_ROOT" in
+ '')
+ rm -f "$DOTEST"/rebase-root ;;
+ *)
+ : >"$DOTEST"/rebase-root ;;
+ esac
echo $ONTO > "$DOTEST"/onto
test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
test t = "$VERBOSE" && : > "$DOTEST"/verbose
diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
index 6359580..29bb6d0 100755
--- a/t/t3412-rebase-root.sh
+++ b/t/t3412-rebase-root.sh
@@ -184,4 +184,92 @@ test_expect_success 'pre-rebase hook stops rebase -i' '
test 0 = $(git rev-list other...stops2 | wc -l)
'
+test_expect_success 'remove pre-rebase hook' '
+ rm -f .git/hooks/pre-rebase
+'
+
+test_expect_success 'set up a conflict' '
+ git checkout master &&
+ echo conflict > B &&
+ git add B &&
+ git commit -m conflict
+'
+
+test_expect_success 'rebase --root with conflict (first part)' '
+ git checkout -b conflict1 other &&
+ test_must_fail git rebase --root --onto master &&
+ git ls-files -u | grep "B$"
+'
+
+test_expect_success 'fix the conflict' '
+ echo 3 > B &&
+ git add B
+'
+
+cat > expect-conflict <<EOF
+6
+5
+4
+3
+conflict
+2
+1
+EOF
+
+test_expect_success 'rebase --root with conflict (second part)' '
+ git rebase --continue &&
+ git log --pretty=tformat:"%s" > conflict1 &&
+ test_cmp expect-conflict conflict1
+'
+
+test_expect_success 'rebase -i --root with conflict (first part)' '
+ git checkout -b conflict2 other &&
+ GIT_EDITOR=: test_must_fail git rebase -i --root --onto master &&
+ git ls-files -u | grep "B$"
+'
+
+test_expect_success 'fix the conflict' '
+ echo 3 > B &&
+ git add B
+'
+
+test_expect_success 'rebase -i --root with conflict (second part)' '
+ git rebase --continue &&
+ git log --pretty=tformat:"%s" > conflict2 &&
+ test_cmp expect-conflict conflict2
+'
+
+sed 's/#/ /g' > expect-conflict-p <<'EOF'
+* Merge branch 'third' into other
+|\##
+| * 6
+* | Merge branch 'side' into other
+|\ \##
+| * | 5
+* | | 4
+|/ /##
+* | 3
+|/##
+* conflict
+* 2
+* 1
+EOF
+
+test_expect_success 'rebase -i -p --root with conflict (first part)' '
+ git checkout -b conflict3 other &&
+ GIT_EDITOR=: test_must_fail git rebase -i -p --root --onto master &&
+ git ls-files -u | grep "B$"
+'
+
+test_expect_success 'fix the conflict' '
+ echo 3 > B &&
+ git add B
+'
+
+test_expect_success 'rebase -i -p --root with conflict (second part)' '
+ git rebase --continue &&
+ git log --graph --topo-order --pretty=tformat:"%s" > conflict3 &&
+ test_cmp expect-conflict-p conflict3
+'
+
test_done
--
1.6.1.469.g6f3d5
^ permalink raw reply related
* [PATCH] Reintegrate script
From: Junio C Hamano @ 2009-01-26 9:03 UTC (permalink / raw)
To: git
In a workflow that uses topic branches heavily, you would need to keep
updating test integration branch(es) all the time. If they are managed
like my 'next' by merging the tips of topics that have grown since the
last integration, it is not so difficult. You only need to review output
from "git branch --no-merged next" to see if there are topics that can and
needs to be merged to 'next'.
But sometimes it is easier to rebuild a test integration branch from
scratch all the time, especially if you do not publish it for others to
build on.
I've been using this script for some time to rebuild jch and pu branches
in my workflow. jch's tip is supposed to always match 'next', but it is
rebuilt from scratch on top of 'master' by merging the same topics that
are in 'next'. You can use the same script in your work.
To use it, you give a commit range base..tip to the script, and you will
see a shell script that uses a series of 'git-merge'. "base" is the more
stable branch that you rebuild your test integration branch on top (in my
case, 'master'), and "tip" is where the tip of the test integration branch
is from the last round (in my case, 'jch' or 'pu').
Then you can run the resulting script, fix conflicted merge and use
'git-commit', and then repeat until all the branches are re-integrated on
top of the base branch.
$ Meta/Reintegrate master..jch >/var/tmp/redo-jch.sh
$ cat /var/tmp/redo-jch.sh
#!/bin/sh
while read branch eh
do
case "$eh" in
"") git merge "$branch" || break ;;
?*) echo >&2 "Eh? $branch $eh"; break ;;
esac
done <<EOF
jc/blame
js/notes
ks/maint-mailinfo-folded~3
tr/previous-branch
EOF
$ git checkout jch
$ git reset --hard master
$ /var/tmp/redo-jch.sh
... if there is conflict, resolve, "git commit" here ...
$ /var/tmp/redo-jch.sh
... repeat until everything is applied.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is taken from my 'todo' branch, which I keep a checkout in Meta/
directory.
Reintegrate | 42 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 42 insertions(+), 0 deletions(-)
create mode 100755 Reintegrate
diff --git a/Reintegrate b/Reintegrate
new file mode 100755
index 0000000..dfdb73e
--- /dev/null
+++ b/Reintegrate
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+merge_msg="Merge branch '\(.*\)'"
+x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+x40="$x40$x40$x40$x40$x40$x40$x40$x40"
+LF='
+'
+
+echo '#!/bin/sh
+while read branch eh
+do
+ case "$eh" in
+ "") git merge "$branch" || break ;;
+ ?*) echo >&2 "Eh? $branch $eh"; break ;;
+ esac
+done <<EOF'
+
+git log --pretty=oneline --first-parent "$1" |
+{
+ series=
+ while read commit msg
+ do
+ other=$(git rev-parse --verify "$commit^2") &&
+ branch=$(expr "$msg" : "$merge_msg") &&
+ tip=$(git rev-parse --verify "refs/heads/$branch" 2>/dev/null) &&
+ merged=$(git name-rev --refs="refs/heads/$branch" "$other" 2>/dev/null) &&
+ merged=$(expr "$merged" : "$x40 \(.*\)") &&
+ test "$merged" != undefined || {
+ other=$(git log -1 --pretty='format:%s' $other) &&
+ merged="$branch :rebased? $other"
+ }
+ if test -z "$series"
+ then
+ series="$merged"
+ else
+ series="$merged$LF$series"
+ fi
+ done
+ echo "$series"
+}
+
+echo 'EOF'
--
1.6.1.1.248.g7f6d2
^ permalink raw reply related
* [PATCH] diff.c: output correct index lines for a split diff
From: Junio C Hamano @ 2009-01-26 8:33 UTC (permalink / raw)
To: Jeff King; +Cc: Pixel, git
In-Reply-To: <7vy6wy8qmm.fsf@gitster.siamese.dyndns.org>
A patch that changes the filetype (e.g. regular file to symlink) of a path
must be split into a deletion event followed by a creation event, which
means that we need to have two independent metainfo lines for each.
However, the code reused the single set of metainfo lines.
As the blob object names recorded on the index lines are usually not used
nor validated on the receiving end, this is not an issue with normal use
of the resulting patch. However, when accepting a binary patch to delete
a blob, git-apply verified that the postimage blob object name on the
index line is 0{40}, hence a patch that deletes a regular file blob that
records binary contents to create a blob with different filetype (e.g. a
symbolic link) failed to apply. "git am -3" also uses the blob object
names recorded on the index line, so it would also misbehave when
synthesizing a preimage tree.
This moves the code to generate metainfo lines around, so that two
independent sets of metainfo lines are used for the split halves.
The fix revealed that one test expected the incorrect behaviour, which is
also fixed here.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
> ...
>> Below is a patch against the test suite that fairly neatly displays the
>> problem. I didn't get a chance to look into actually fixing it, though
>> (I'm not even sure the problem is in apply, and not in the generated
>> patch).
>
> The generated diff is wrong.
>
> A filepair that changes type must be split into deletion followed by
> creation, which means the "index" line should say 0{40} on the right hand
> side for the first half and then 0{40} on the left hand side for the
> second half. The patch generated by this step:
>
>> +test_expect_success 'create patch' '
>> + git diff-tree --binary HEAD^ HEAD >patch
>> +'
>
> However says the blob contents change from "\0" to "file" on both.
>
> This is because diff.c::run_diff() computes "index" only once and reuses
> it for both halves.
I did not include your new test script here; perhaps we can add it to an
existing typechange diff/apply test, like t4114?
diff.c | 146 +++++++++++++++++++++++++---------------------
t/t4030-diff-textconv.sh | 2 +-
2 files changed, 81 insertions(+), 67 deletions(-)
diff --git a/diff.c b/diff.c
index 972b3da..7e18364 100644
--- a/diff.c
+++ b/diff.c
@@ -2081,16 +2081,86 @@ static void run_external_diff(const char *pgm,
}
}
+static int similarity_index(struct diff_filepair *p)
+{
+ return p->score * 100 / MAX_SCORE;
+}
+
+static void fill_metainfo(struct strbuf *msg,
+ const char *name,
+ const char *other,
+ struct diff_filespec *one,
+ struct diff_filespec *two,
+ struct diff_options *o,
+ struct diff_filepair *p)
+{
+ strbuf_init(msg, PATH_MAX * 2 + 300);
+ switch (p->status) {
+ case DIFF_STATUS_COPIED:
+ strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
+ strbuf_addstr(msg, "\ncopy from ");
+ quote_c_style(name, msg, NULL, 0);
+ strbuf_addstr(msg, "\ncopy to ");
+ quote_c_style(other, msg, NULL, 0);
+ strbuf_addch(msg, '\n');
+ break;
+ case DIFF_STATUS_RENAMED:
+ strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
+ strbuf_addstr(msg, "\nrename from ");
+ quote_c_style(name, msg, NULL, 0);
+ strbuf_addstr(msg, "\nrename to ");
+ quote_c_style(other, msg, NULL, 0);
+ strbuf_addch(msg, '\n');
+ break;
+ case DIFF_STATUS_MODIFIED:
+ if (p->score) {
+ strbuf_addf(msg, "dissimilarity index %d%%\n",
+ similarity_index(p));
+ break;
+ }
+ /* fallthru */
+ default:
+ /* nothing */
+ ;
+ }
+ if (one && two && hashcmp(one->sha1, two->sha1)) {
+ int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
+
+ if (DIFF_OPT_TST(o, BINARY)) {
+ mmfile_t mf;
+ if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
+ (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
+ abbrev = 40;
+ }
+ strbuf_addf(msg, "index %.*s..%.*s",
+ abbrev, sha1_to_hex(one->sha1),
+ abbrev, sha1_to_hex(two->sha1));
+ if (one->mode == two->mode)
+ strbuf_addf(msg, " %06o", one->mode);
+ strbuf_addch(msg, '\n');
+ }
+ if (msg->len)
+ strbuf_setlen(msg, msg->len - 1);
+}
+
static void run_diff_cmd(const char *pgm,
const char *name,
const char *other,
const char *attr_path,
struct diff_filespec *one,
struct diff_filespec *two,
- const char *xfrm_msg,
+ struct strbuf *msg,
struct diff_options *o,
- int complete_rewrite)
+ struct diff_filepair *p)
{
+ const char *xfrm_msg = NULL;
+ int complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;
+
+ if (msg) {
+ fill_metainfo(msg, name, other, one, two, o, p);
+ xfrm_msg = msg->len ? msg->buf : NULL;
+ }
+
if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
pgm = NULL;
else {
@@ -2130,11 +2200,6 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
hashclr(one->sha1);
}
-static int similarity_index(struct diff_filepair *p)
-{
- return p->score * 100 / MAX_SCORE;
-}
-
static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
{
/* Strip the prefix but do not molest /dev/null and absolute paths */
@@ -2148,13 +2213,11 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
{
const char *pgm = external_diff();
struct strbuf msg;
- char *xfrm_msg;
struct diff_filespec *one = p->one;
struct diff_filespec *two = p->two;
const char *name;
const char *other;
const char *attr_path;
- int complete_rewrite = 0;
name = p->one->path;
other = (strcmp(name, p->two->path) ? p->two->path : NULL);
@@ -2164,83 +2227,34 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
if (DIFF_PAIR_UNMERGED(p)) {
run_diff_cmd(pgm, name, NULL, attr_path,
- NULL, NULL, NULL, o, 0);
+ NULL, NULL, NULL, o, p);
return;
}
diff_fill_sha1_info(one);
diff_fill_sha1_info(two);
- strbuf_init(&msg, PATH_MAX * 2 + 300);
- switch (p->status) {
- case DIFF_STATUS_COPIED:
- strbuf_addf(&msg, "similarity index %d%%", similarity_index(p));
- strbuf_addstr(&msg, "\ncopy from ");
- quote_c_style(name, &msg, NULL, 0);
- strbuf_addstr(&msg, "\ncopy to ");
- quote_c_style(other, &msg, NULL, 0);
- strbuf_addch(&msg, '\n');
- break;
- case DIFF_STATUS_RENAMED:
- strbuf_addf(&msg, "similarity index %d%%", similarity_index(p));
- strbuf_addstr(&msg, "\nrename from ");
- quote_c_style(name, &msg, NULL, 0);
- strbuf_addstr(&msg, "\nrename to ");
- quote_c_style(other, &msg, NULL, 0);
- strbuf_addch(&msg, '\n');
- break;
- case DIFF_STATUS_MODIFIED:
- if (p->score) {
- strbuf_addf(&msg, "dissimilarity index %d%%\n",
- similarity_index(p));
- complete_rewrite = 1;
- break;
- }
- /* fallthru */
- default:
- /* nothing */
- ;
- }
-
- if (hashcmp(one->sha1, two->sha1)) {
- int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
-
- if (DIFF_OPT_TST(o, BINARY)) {
- mmfile_t mf;
- if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
- (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
- abbrev = 40;
- }
- strbuf_addf(&msg, "index %.*s..%.*s",
- abbrev, sha1_to_hex(one->sha1),
- abbrev, sha1_to_hex(two->sha1));
- if (one->mode == two->mode)
- strbuf_addf(&msg, " %06o", one->mode);
- strbuf_addch(&msg, '\n');
- }
-
- if (msg.len)
- strbuf_setlen(&msg, msg.len - 1);
- xfrm_msg = msg.len ? msg.buf : NULL;
-
if (!pgm &&
DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
(S_IFMT & one->mode) != (S_IFMT & two->mode)) {
- /* a filepair that changes between file and symlink
+ /*
+ * a filepair that changes between file and symlink
* needs to be split into deletion and creation.
*/
struct diff_filespec *null = alloc_filespec(two->path);
run_diff_cmd(NULL, name, other, attr_path,
- one, null, xfrm_msg, o, 0);
+ one, null, &msg, o, p);
free(null);
+ strbuf_release(&msg);
+
null = alloc_filespec(one->path);
run_diff_cmd(NULL, name, other, attr_path,
- null, two, xfrm_msg, o, 0);
+ null, two, &msg, o, p);
free(null);
}
else
run_diff_cmd(pgm, name, other, attr_path,
- one, two, xfrm_msg, o, complete_rewrite);
+ one, two, &msg, o, p);
strbuf_release(&msg);
}
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 2f27a0b..a3f0897 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -104,7 +104,7 @@ cat >expect.typechange <<'EOF'
-1
diff --git a/file b/file
new file mode 120000
-index ad8b3d2..67be421
+index 0000000..67be421
--- /dev/null
+++ b/file
@@ -0,0 +1 @@
--
1.6.1.1.248.g7f6d2
^ permalink raw reply related
* [PATCH (v2)] Mention "local convention" rule in the CodingGuidelines
From: Nanako Shiraishi @ 2009-01-26 8:32 UTC (permalink / raw)
To: git; +Cc: Sverre Rabbelier
The document suggests to imitate the existing code, but didn't
say which existing code it should imitate. This clarifies.
Signed-off-by: しらいしななこ <nanako3@lavabit.com>
---
Quoting Junio C Hamano <gitster@pobox.com>:
> It is always preferable to match the _local_ convention. I'd expect a new
> script added to git suite to match my preference (the one I showed you in
> my comments to you that is used in git-am, which is what you suggested
> above), but I'd expect a modification to mergetool to match the style
> mergetool already uses.
Sverre fixed some typo for me.
Documentation/CodingGuidelines | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index f628c1f..664c6c2 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -21,8 +21,13 @@ code. For git in general, three rough rules are:
As for more concrete guidelines, just imitate the existing code
(this is a good guideline, no matter which project you are
-contributing to). But if you must have a list of rules,
-here they are.
+contributing to). It is always preferable to match the _local_
+convention. New code added to git suite is expected to match
+the overall style of existing code. Modifications to existing
+code is expected to match the style the surrounding code already
+uses (even if it doesn't match the overall style of existing code).
+
+But if you must have a list of rules, here they are.
For shell scripts specifically (not exhaustive):
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply related
* Re: [PATCH] http-push: refactor request url creation
From: Daniel Stenberg @ 2009-01-26 8:02 UTC (permalink / raw)
To: Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0901251752p5b34c053pb24dce8a35b06fce@mail.gmail.com>
On Mon, 26 Jan 2009, Ray Chuan wrote:
>>> - curl_easy_setopt(slot->curl, CURLOPT_URL, url);
>>> + curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
>>
>> The original code gave a separate "url" to setop() but this gives the same
>> string. Does curl_easy_setop() copies the given string away? IOW is this
>> change safe?
>
> curl strdup's it, so this is safe.
I'm not sure what the oldest possibly libcurl version git can deal with, but
here's a related quote from the curl_easy_setopt man page:
NOTE: before 7.17.0 strings were not copied. Instead the user was
forced keep them available until libcurl no longer needed them.
--
/ daniel.haxx.se
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #06; Sat, 24)
From: Johannes Sixt @ 2009-01-26 7:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Steffen Prohaska
In-Reply-To: <7v8wp0kmj4.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> * sp/runtime-prefix (Sun Jan 18 13:00:15 2009 +0100) 7 commits
> - Windows: Revert to default paths and convert them by
> RUNTIME_PREFIX
> - Compute prefix at runtime if RUNTIME_PREFIX is set
> - Modify setup_path() to only add git_exec_path() to PATH
> - Add calls to git_extract_argv0_path() in programs that call
> git_config_*
> - git_extract_argv0_path(): Move check for valid argv0 from caller
> to callee
> - Refactor git_set_argv0_path() to git_extract_argv0_path()
> - Move computation of absolute paths from Makefile to runtime (in
> preparation for RUNTIME_PREFIX)
>
> We should move this to 'next' soon with J6t's blessing.
I've been using this series for a few days now without problems:
Acked-by: Johannes Sixt <j6t@kdbg.org>
-- Hannes
^ permalink raw reply
* Re: bug: transform a binary file into a symlink in one commit => invalid binary patch
From: Junio C Hamano @ 2009-01-26 7:37 UTC (permalink / raw)
To: Jeff King; +Cc: Pixel, git
In-Reply-To: <20090126003556.GA19368@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Jan 23, 2009 at 01:25:30PM +0100, Pixel wrote:
>
>> i hit a bug (git 1.6.1): when you transform a binary file into a
>> symlink in one commit, the binary patch can't be used in "git apply".
>> Is it a known issue?
>
> Not that I know of.
>
> Below is a patch against the test suite that fairly neatly displays the
> problem. I didn't get a chance to look into actually fixing it, though
> (I'm not even sure the problem is in apply, and not in the generated
> patch).
The generated diff is wrong.
A filepair that changes type must be split into deletion followed by
creation, which means the "index" line should say 0{40} on the right hand
side for the first half and then 0{40} on the left hand side for the
second half. The patch generated by this step:
> +test_expect_success 'create patch' '
> + git diff-tree --binary HEAD^ HEAD >patch
> +'
However says the blob contents change from "\0" to "file" on both.
This is because diff.c::run_diff() computes "index" only once and reuses
it for both halves.
^ permalink raw reply
* Re: [PATCH 08/10] run test suite without dashed git-commands in PATH
From: Matthew Ogilvie @ 2009-01-26 6:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901250255250.14855@racer>
Hi,
On Sun, Jan 25, 2009 at 02:59:53AM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Sat, 24 Jan 2009, Matthew Ogilvie wrote:
>
> > .gitignore | 1 +
> > Makefile | 42 +++++++++++++++++++++++++++++++-----------
> > t/test-lib.sh | 14 +++++++++++++-
> > test-bin-wrapper.sh | 12 ++++++++++++
> > 4 files changed, 57 insertions(+), 12 deletions(-)
> > create mode 100644 test-bin-wrapper.sh
>
> I am strongly opposed to a patch this big, just for something as 3rd class
> as CVS server faking. We already have a big fallout from all that bending
> over for Windows support, and I do not like it at all.
>
> Note: I do not even have to look further than the diffstat to see that it
> is wrong.
>
> The point is: if cvsserver wants to pretend that it is in a fake bin where
> almost none of the other Git programs are, fine, let's do that _in the
> test for cvsserver_.
>
> Let's not fsck up the whole test suite just for one user.
>
> Ciao,
> Dscho
Since by default git is installed such that most of the dashed-form
commands are not in a user's default PATH, my thought was that
it would make sense for the test suite to mimick that environment
as much as possible. This could detect regressions in any
installed/tested git script that erroneously assumes the dashed
form commands are in the PATH, not just git-cvsserver.
I can think of ways it could be made smaller, but they seem uglier than
the current patch to me:
1. Perhaps just list the executables for the fake bin directory
separately, but then it is all too easy for the list to get out of sync
with what the final install environment will be.
2. Perhaps strip off the $X's (.exe on windows; currently empty
elsewhere) from the words of the existing variables, in the rule
for building the "fake bin" directory. But generally I'ld rather
not assume that pattern-matching replacement of
$X's will never conflict with a part of a script name.
3. Perhaps just use symlinks or hardlinks instead of a wrapper
script. This might have some promise, except that links are more
likely to fail on windows, and the wrappers generally give you
more flexibility for testing odd scenarios.
4. The test-bin-wrapper.sh script does not actually need to
set environment variables (GIT_EXEC_DIT and templates) for
purposes of this patch. But my thought was that in this form
you could run things straight out of the test-bin directory
to manually try out new code without needing to actually install
a build or mess with the environment variables yourself. It could
also be extended to handle other global wrapper needs
relatively easily, such as valgrind.
Any other ideas?
--
Matthew Ogilvie [mmogilvi_git@miniinfo.net]
^ permalink raw reply
* Re: [PATCH] git-svn: add --ignore-paths option for fetching
From: Junio C Hamano @ 2009-01-26 6:28 UTC (permalink / raw)
To: Eric Wong; +Cc: public_vi, Thomas Rast, git
In-Reply-To: <20090126011847.GA8703@dcvr.yhbt.net>
Eric Wong <normalperson@yhbt.net> writes:
> Thanks Vitaly, acked and pushed out with minor fixes to
> git://git.bogomips.org/git-svn.git
Pulled; thanks.
^ permalink raw reply
* Re: [PATCH 1/3] Documentation: simplify refspec format description
From: Junio C Hamano @ 2009-01-26 6:24 UTC (permalink / raw)
To: Anders Melchiorsen; +Cc: git
In-Reply-To: <1232927133-30377-2-git-send-email-mail@cup.kalibalik.dk>
Anders Melchiorsen <mail@cup.kalibalik.dk> writes:
> diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt
> index ebdd948..820c140 100644
> --- a/Documentation/pull-fetch-param.txt
> +++ b/Documentation/pull-fetch-param.txt
> @@ -5,10 +5,10 @@
> of a remote (see the section <<REMOTES,REMOTES>> below).
>
> <refspec>::
> - The canonical format of a <refspec> parameter is
> - `+?<src>:<dst>`; that is, an optional plus `{plus}`, followed
> - by the source ref, followed by a colon `:`, followed by
> - the destination ref.
> + The format of a <refspec> parameter is an optional plus
> + `{plus}`, followed by the source ref <src>, followed
> + by a colon `:`, followed by the destination ref <dst>.
> + Find various forms of refspecs in examples section.
> +
> The remote ref that matches <src>
> is fetched, and if <dst> is not empty string, the local
I think this is *much* nicer, but I do not think git-fetch.txt has
examples to fall back on.
The patch to git-push.txt would not have this issue; the exmaple is there
in the page itself.
But I think it might be even better to briefly describe what it means,
like this patch on top of yours does to git-push.txt. The fetch/pull side
already has the corresponding description immediately after that, so I'd
suggest just removing the reference to non-existing examples section.
I found your 2/3 and 3/3 good improvements.
Documentation/git-push.txt | 3 ++-
Documentation/pull-fetch-param.txt | 3 +--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git i/Documentation/git-push.txt w/Documentation/git-push.txt
index 3fd4bbb..ea45935 100644
--- i/Documentation/git-push.txt
+++ w/Documentation/git-push.txt
@@ -36,7 +36,8 @@ OPTIONS
The format of a <refspec> parameter is an optional plus
`{plus}`, followed by the source ref <src>, followed
by a colon `:`, followed by the destination ref <dst>.
- Find various forms of refspecs in examples section.
+ It is used to specify with what <src> object the <dst> ref
+ in the remote repository is to be updated.
+
The <src> side represents the source branch (or arbitrary
"SHA1 expression", such as `master~4` (four parents before the
diff --git i/Documentation/pull-fetch-param.txt w/Documentation/pull-fetch-param.txt
index 820c140..f9811f2 100644
--- i/Documentation/pull-fetch-param.txt
+++ w/Documentation/pull-fetch-param.txt
@@ -8,12 +8,11 @@
The format of a <refspec> parameter is an optional plus
`{plus}`, followed by the source ref <src>, followed
by a colon `:`, followed by the destination ref <dst>.
- Find various forms of refspecs in examples section.
+
The remote ref that matches <src>
is fetched, and if <dst> is not empty string, the local
ref that matches it is fast forwarded using <src>.
-Again, if the optional plus `+` is used, the local ref
+If the optional plus `+` is used, the local ref
is updated even if it does not result in a fast forward
update.
+
^ permalink raw reply related
* Re: [PATCH 1/2] rebase -i --root: simplify code
From: Junio C Hamano @ 2009-01-26 5:54 UTC (permalink / raw)
To: Thomas Rast; +Cc: Johannes Schindelin, git
In-Reply-To: <200901260053.06315.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Thomas Rast wrote:
>> test ! -s "$DOTEST"/upstream && REBASE_ROOT=t
>
> Actually, I think that test never worked (and it's clearly my fault).
>
> The corresponding 'echo $UPSTREAM > "$DOTEST"/upstream' just expanded
> to 'echo > ...', resulting in a file containing a single newline, but
> never a zero-length file. Duh.
Since you never use the value stored in "$DOTEST/upstream" for anything
else anyway, how about doing something like this instead? It would make
the meaning of the file used as a state variable much clearer.
It may break hooks and outside scripts that look at $DOTEST/upstream. I
didn't check.
The hunk in the middle is to protect you against an environment variable
UPSTREAM the user may have before starting "rebase -i". There could be
other state variables you added in recent commit d911d14 (rebase -i: learn
to rebase root commit, 2009-01-02) that needs similar protection. Please
check.
git-rebase--interactive.sh | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git c/git-rebase--interactive.sh w/git-rebase--interactive.sh
index 21ac20c..17cf0e5 100755
--- c/git-rebase--interactive.sh
+++ w/git-rebase--interactive.sh
@@ -456,7 +456,7 @@ get_saved_options () {
test -d "$REWRITTEN" && PRESERVE_MERGES=t
test -f "$DOTEST"/strategy && STRATEGY="$(cat "$DOTEST"/strategy)"
test -f "$DOTEST"/verbose && VERBOSE=t
- test ! -s "$DOTEST"/upstream && REBASE_ROOT=t
+ test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
}
while test $# != 0
@@ -585,6 +585,7 @@ first and then run 'git rebase --continue' again."
test -z "$ONTO" && ONTO=$UPSTREAM
shift
else
+ UPSTREAM=
UPSTREAM_ARG=--root
test -z "$ONTO" &&
die "You must specify --onto when using --root"
@@ -611,7 +612,12 @@ first and then run 'git rebase --continue' again."
echo "detached HEAD" > "$DOTEST"/head-name
echo $HEAD > "$DOTEST"/head
- echo $UPSTREAM > "$DOTEST"/upstream
+ case "$REBASE_ROOT" in
+ '')
+ rm -f "$DOTEST"/rebase-root ;;
+ *)
+ : >"$DOTEST"/rebase-root ;;
+ esac
echo $ONTO > "$DOTEST"/onto
test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
test t = "$VERBOSE" && : > "$DOTEST"/verbose
^ permalink raw reply related
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-26 3:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Keith Cascio, git
In-Reply-To: <7v4ozmbunl.fsf@gitster.siamese.dyndns.org>
On Sun, Jan 25, 2009 at 07:42:06PM -0800, Junio C Hamano wrote:
> We seem to think the same way these days, so I do not have very much to
> add on top of what you already said. I'll fix one typo, though.
Heh. Am I corrupting you, or you me?
> > But this can break down in two ways:
> >
> > 1. Sometimes we blur the line of plumbing and porcelain, where
> > functionality is available only through plumbing. For example,
>
> s/through plumbing/through Porcelain/.
Oops, yes, thank you. That was what I meant.
-Peff
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Junio C Hamano @ 2009-01-26 3:42 UTC (permalink / raw)
To: Jeff King; +Cc: Keith Cascio, git
In-Reply-To: <20090126031206.GB14277@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
We seem to think the same way these days, so I do not have very much to
add on top of what you already said. I'll fix one typo, though.
> But this can break down in two ways:
>
> 1. Sometimes we blur the line of plumbing and porcelain, where
> functionality is available only through plumbing. For example,
s/through plumbing/through Porcelain/.
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Junio C Hamano @ 2009-01-26 3:38 UTC (permalink / raw)
To: Jeff King; +Cc: Keith Cascio, Johannes Schindelin, git
In-Reply-To: <20090126031820.GC14277@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I think what you will end up with is a list in gitk of "these
> particular options are known good for passing through". And that doesn't
> really need tool support from git-core. It's up to each script how much
> it wants to protect the user.
I tend to agree. Also at the same time this does not have to contradict
with what Keith wants to do. gitk just needs to learn to peek into
diff.primer, and use the safe ones while discarding others. A tool
support is already there in the form of git-config to do this, though.
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Junio C Hamano @ 2009-01-26 3:36 UTC (permalink / raw)
To: Keith Cascio; +Cc: git
In-Reply-To: <alpine.GSO.2.00.0901251345240.12651@kiwi.cs.ucla.edu>
Keith Cascio <keith@CS.UCLA.EDU> writes:
> I agree opt-in is always better with new grammar/semantics. However, the
> constraint I was trying to live inside is: if I call "git diff" on the command
> line with no options at all
"git diff" is a Porcelain.
> Worth considering: I believe Git users who've written their own scripts are not
> the type to use diff.primer blindly, if at all.
Perhaps this nonsense comes from your misunderstanding on the line between
the plumbing and the Porcelain.
Users of git, including me, would love to be able to use default options
in our $HOME/.gitconfig file when using "git diff" interactively, but will
refuse to see our scripts that we wrote using "git diff-files" and "git
diff-index" broken, because the reason we explicitly used these plumbing
commands is to avoid getting broken with a change from the underlying
version of git. That's the whole point of output stability for the
plumbing.
If you want to be able to use -w or -b (or --color) in git-gui, you must
first vet the script to see if it can sanely operate on the output from
git-diff-index with such options, and after it is determined that it is
safe, it should give its users a way to pass that to the underlying
plumbing, or picked up such options from the configuration (perhaps using
the same diff.primer configuration).
This has to be a conscious opt-in process per script.
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-26 3:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Keith Cascio, Johannes Schindelin, git
In-Reply-To: <7v1vuqdcjp.fsf@gitster.siamese.dyndns.org>
On Sun, Jan 25, 2009 at 06:30:18PM -0800, Junio C Hamano wrote:
> It would be a good addition to our toolset if scripts like gitk can
> declare which options and features are safe to accept from the end user to
> pass down to the plumbing tools. "git rev-parse", which lets the script
> sift between options that are meant to affect ancestry traversal and the
> ones that are for other (primarily diff family) commands, does not do
> anything fancy like that, but it would be a logical place to do this sort
> of thing.
I'm not sure there is a good way of doing this at a less fine-grained
level than "each option". That is, how can git-core, without knowing how
the script will use the output, classify options in groups according to
how the script will react to them?
It seems like "--since" is innocent enough for gitk. It just limits the
commits shown. So maybe it goes into the "ancestry traversal" list. But
is that whole list safe? "--reverse" isn't, but I would have put it in
the same list.
So I think what you will end up with is a list in gitk of "these
particular options are known good for passing through". And that doesn't
really need tool support from git-core. It's up to each script how much
it wants to protect the user.
But if you are proposing that some config options can be "enabled" by
scripts selectively, then I think that does need tool support. Keith's
"primer" example will be parsed by git, not by whatever script is
calling it. So we would need to feed it some list of "these are the OK
options".
-Peff
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-26 3:12 UTC (permalink / raw)
To: Keith Cascio; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.GSO.2.00.0901251345240.12651@kiwi.cs.ucla.edu>
On Sun, Jan 25, 2009 at 06:40:02PM -0800, Keith Cascio wrote:
> I agree opt-in is always better with new grammar/semantics. However,
> the constraint I was trying to live inside is: if I call "git diff" on
> the command line with no options at all, then primer active. Yet
> perhaps that's not possible, and the only way to do primer is to
> require opt-in spelled "--primer". Then I can tell bash to alias
> 'gitdiff' as 'git diff --primer' and use that on the command line.
What's the point of aliasing something that isn't "git diff" to "git
diff --primer"? At that point, couldn't you just do away with --primer
entirely and alias "gitdiff" to "git diff --whatever --your --primer
--options --are"?
Anyway, I think that isn't necessary. We _do_ have a mechanism to handle
this already: some commands are plumbing, and must have stable
interfaces, and some commands are porcelain, and can do your magic
automatically. For example, gitk doesn't actually call "git diff"; it
calls "git diff-tree", "git diff-index", etc.
So if you just want this from the command line, then I think it is safe
to have "git diff" always respect "diff.primer", and scripts shouldn't
be impacted.
But this can break down in two ways:
1. Sometimes we blur the line of plumbing and porcelain, where
functionality is available only through plumbing. For example,
gitweb until recently called "git diff" because there is no other
way to diff two arbitrary blobs. But the solution there is, I
think, to make that functionality available through plumbing. Not
to disallow enhancements to porcelain.
2. When you want a script to take advantage of porcelain-like options,
the situation is much more difficult (and this is what Junio was
talking about in his last mail).
What I think is sane is:
a. You grow new feature X.
b. Porcelain takes advantage of any config that asks us to use X.
c. Plumbing does _not_ respect such config, but will respect
command line options.
d. Scripts control which command line options they use; when the
script writer decides feature X will not interfere (either
because it is harmless to the script's use, or because the
script is enhanced to handle the new behavior), then it can
pass an "--allow-X" command line option.
And of course that has two disadvantages (and I'm running out of
numbering schemes):
I. You have to wait for the script to be updated before you can
start using X, even if _you_ know that it's harmless.
II. Point (d) is not always true. Junio mentioned the fact that
gitk passes command line parameters blindly to rev-list, which
is potentially unsafe. Up until now, our attitude has been "if
it hurts, don't do it". In other words, if you call "gitk
--reverse" and it looks ugly, then it is your fault. :)
-Peff
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-26 2:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1vurf7lq.fsf@gitster.siamese.dyndns.org>
On Sun, 25 Jan 2009, Junio C Hamano wrote:
> Your Subject is good; in a shortlog output that will be taken 3 months
> down the road, it will still tell us what this patch was about, among 100
> other patches that are about different topics.
Thanks, I value the encouragement.
> The proposed commit log message describes what the patch does, but it does
> not explain what problem it solves, nor why the approach the patch takes
> to solve that problem is good. Lines that are too short and too dense
> without paragraph breaks do not help readability either.
Noted, future versions will be better.
> You can work around the backward incompatibility you are introducing for known
> users that you broke with your patch, by including updates to them, and that
> is what your patches to git-gui and gitk are, but that is a sure sign that the
> approach is flawed.
>
> The point of lowlevel plumbing (e.g. diff-{files,index,tree}) is to give
> people's scripts an interface that they can rely on. It is not about giving a
> magic interface that all the users are somehow magically upgraded without
> change, when the underlying git is upgraded.
I believe, as I know you do as well, concept is the right way to approach code.
I believe in the power of concept, and I agree that porcelain/plumbing is a good
and powerful concept, and should not be violated.
> If a script X does not use "ignore whitespace" without an explicit request
> from the end user when it runs diff-tree internally, installing a new
> version of diff-tree should *NOT* magically make script X to run it with
> "ignore whitespace", because you do not know what the script X uses
> diff-tree output for and how, even when the end user sets diff.primer to
> get "ignore whitespace" applied to his command line invocation of "git
> diff" Porcelain. Imagine a case where the operation of that script X
> relies on seeing at least two context lines around the hunk in order to
> correctly parse textual diff output from "diff-index -p", and the user
> sets "-U1" in diff.primer --- you would break the script and it is not
> fair to blame the script for not explicitly passing -U3 and relying on the
> default.
>
> Scriptability by definition means you do not know how scripts written by
> people around plumbing use the output; I do not think you can sensibly say
> "this should not be turned on in a machine friendly output, but this is
> safe to use".
>
> I would not be opposed to an enhancement to the plumbing that the scripts
> can use to say "I am willing to take any option (or perhaps "these
> options") given to me via diff.primer". Some scripts may want to be just
> a pass-thru of whatever the underlying git-diff-* command outputs, and it
> may be a handy way to magically upgrade them to allow their invocation of
> lowlevel plumbing to be affected by what the end-user configured. But
> that magic upgrade has to be an opt/in process.
I agree opt-in is always better with new grammar/semantics. However, the
constraint I was trying to live inside is: if I call "git diff" on the command
line with no options at all, then primer active. Yet perhaps that's not
possible, and the only way to do primer is to require opt-in spelled "--primer".
Then I can tell bash to alias 'gitdiff' as 'git diff --primer' and use that on
the command line. And I could patch git-gui and gitk to call 'git diff --primer
--no-color'. Note that would still require the same magnitude of change to the
git-gui and gitk Tcl code as occurs with my v1 patch.
It would be really cool if there was a way
to make this work without "--primer" !!!
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Worth considering: I believe Git users who've written their own scripts are not
the type to use diff.primer blindly, if at all. If they did use it, they'd be
very thoughtful about its consequences for their existing scripts. Based on
that belief, and the fact that I already protected every script in Git core, and
that I could work with the authors of contrib to do the same there, I have a
very high degree of confidence that no existing script would be broken the day
you release diff.primer, if ever :(
Because an enhancement *could* break existing scripts is not sufficient grounds
to declare it worthless. My gut tells me there is a sane way to do this, but I
need more familiarity with the "literature" to discover it.
I see evidence there is already anxiety surrounding the necessity of
"machine-friendliness", Cf.
http://article.gmane.org/gmane.comp.version-control.git/107006
I think my DIFF_MACHINE_FRIENDLY() cpp macro with --machine-friendly command
line option does a nice job of making that explicit, so we could all just relax!
When a monster is lurking under the bridge threatening the kingdom, put it to
death once and for all! Rather than relying on an elaborate system of
conventions prescribing how to tip-toe acrosss the bridge just the right way
while pretending the monster isn't there.
> There are funny indentation to align the same variable names on two
> adjacent lines and such; please don't.
You're right. I'm sorry. I'll fix all style to adhere to local convention.
-- Keith
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-26 2:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <7v1vuqdcjp.fsf@gitster.siamese.dyndns.org>
On Sun, 25 Jan 2009, Junio C Hamano wrote:
> I suspect it is pretty much orthogonal to the "use user's default without
> being told from the command line", but it might be a worthy goal to introduce
> a mechanism for the scripts to accept "safe" default options from the end user
> while rejecting undesirable ones that would interfere with the way it uses
> plumbing.
>
> For example, gitk drives "git rev-list" and many options you give from the
> command line (e.g. "gitk --all --simplify-merges -- drivers/") are passed to
> the underlying plumbing.
>
> This is a double edged sword. When we add new features to git-rev-list, (e.g.
> --simplify-merges or --simplify-by-decoration are fairly recent inventions
> that did not exist when gitk was written originally), some of them can be
> safely passed and automagically translates to a new feature in gitk.
> However, use of some options (e.g. --reverse) breaks the assumption the tool
> makes on the output from the underlying plumbing and should not be accepted
> from the end-user.
>
> It would be a good addition to our toolset if scripts like gitk can declare
> which options and features are safe to accept from the end user to pass down
> to the plumbing tools. "git rev-parse", which lets the script sift between
> options that are meant to affect ancestry traversal and the ones that are for
> other (primarily diff family) commands, does not do anything fancy like that,
> but it would be a logical place to do this sort of thing.
>
> And it is not limited to "scripts" use. A recent topic on rejecting colouring
> options from being given to format-patch would also be helped with such a
> mechanism if it is available to builtins.
>
> Just an idle thought.
Yes yes yes yes!!!!! I've been working on a response to your previous message,
in which I address exactly this possibility. Coming soon.
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Junio C Hamano @ 2009-01-26 2:30 UTC (permalink / raw)
To: Keith Cascio; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <7v1vurf7lq.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Scriptability by definition means you do not know how scripts written by
> people around plumbing use the output; I do not think you can sensibly say
> "this should not be turned on in a machine friendly output, but this is
> safe to use".
>
> I would not be opposed to an enhancement to the plumbing that the scripts
> can use to say "I am willing to take any option (or perhaps "these
> options") given to me via diff.primer". Some scripts may want to be just
> a pass-thru of whatever the underlying git-diff-* command outputs, and it
> may be a handy way to magically upgrade them to allow their invocation of
> lowlevel plumbing to be affected by what the end-user configured. But
> that magic upgrade has to be an opt/in process.
I suspect it is pretty much orthogonal to the "use user's default without
being told from the command line", but it might be a worthy goal to
introduce a mechanism for the scripts to accept "safe" default options
from the end user while rejecting undesirable ones that would interfere
with the way it uses plumbing.
For example, gitk drives "git rev-list" and many options you give from the
command line (e.g. "gitk --all --simplify-merges -- drivers/") are passed
to the underlying plumbing.
This is a double edged sword. When we add new features to git-rev-list,
(e.g. --simplify-merges or --simplify-by-decoration are fairly recent
inventions that did not exist when gitk was written originally), some of
them can be safely passed and automagically translates to a new feature in
gitk. However, use of some options (e.g. --reverse) breaks the assumption
the tool makes on the output from the underlying plumbing and should not
be accepted from the end-user.
It would be a good addition to our toolset if scripts like gitk can
declare which options and features are safe to accept from the end user to
pass down to the plumbing tools. "git rev-parse", which lets the script
sift between options that are meant to affect ancestry traversal and the
ones that are for other (primarily diff family) commands, does not do
anything fancy like that, but it would be a logical place to do this sort
of thing.
And it is not limited to "scripts" use. A recent topic on rejecting
colouring options from being given to format-patch would also be helped
with such a mechanism if it is available to builtins.
Just an idle thought.
^ permalink raw reply
* Re: [PATCH] gitweb: check if-modified-since for feeds
From: Jakub Narebski @ 2009-01-26 2:18 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <1232923370-4427-2-git-send-email-giuseppe.bilotta@gmail.com>
Should be "[PATCH 2/2]" or similar, just in case.
On Sun, 25 Jun 2009, Giuseppe Bilotta wrote:
> Offering Last-modified header
And skipping generating the body if client uses 'HEAD' request to
get only Last-Modified header.
> for feeds is only half the work: we should
> also check that same date against If-modified-since, and bail out early
> with 304 Not Modified.
Lacks signoff.
> ---
> gitweb/gitweb.perl | 20 +++++++++++++++++++-
> 1 files changed, 19 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8c49c75..0a5d229 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6015,7 +6015,25 @@ sub git_feed {
> }
> if (defined($commitlist[0])) {
> %latest_commit = %{$commitlist[0]};
> - %latest_date = parse_date($latest_commit{'committer_epoch'});
> + my $latest_epoch = $latest_commit{'committer_epoch'};
> + %latest_date = parse_date($latest_epoch);
> + my $if_modified = $cgi->http('IF_MODIFIED_SINCE');
> + if (defined $if_modified) {
> + my $since;
> + if (eval { require HTTP::Date; 1; }) {
> + $since = HTTP::Date::str2time($if_modified);
> + } elsif (eval { require Time::ParseDate; 1; }) {
> + $since = Time::ParseDate::parsedate($if_modified, GMT => 1);
> + }
I'd really like to fallback on hand-parsing, as we have to parse date
in well defined HTTP-date format (RFC-1123, update to RFC-822), which
I think is what we send in Last-Modified header (or is it RFC-2822?).
But that might be too much work. I like the checking for modules,
and the fallback cascade, but could you explain why in this order?
> + if (defined $since && $latest_epoch <= $since) {
> + print $cgi->header(
> + -type => $content_type,
> + -charset => 'utf-8',
> + -last_modified => $latest_date{'rfc2822'},
> + -status => 304);
I think we spell HTTP status messages in full (even if it is hidden
in die_error subroutine), i.e.
+ -status => '304 Not Modified');
> + return;
> + }
> + }
> print $cgi->header(
> -type => $content_type,
> -charset => 'utf-8',
> --
> 1.5.6.5
>
>
P.S. It would be nice to have this mechanism (responding to
cache-control headers such as If-Modified-Since) for all of gitweb,
but I guess it is most critical for feeds, which are _polled_.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: diff settings
From: Ted Pavlic @ 2009-01-26 2:06 UTC (permalink / raw)
To: Keith Cascio; +Cc: Teemu Likonen, git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <alpine.GSO.2.00.0901251307030.12651@kiwi.cs.ucla.edu>
>> Mercurial has a "defaults.*" that does exactly what you want. That is, you can
>> [defaults]
>> diff = -w
>> and "hg diff" will automatically do "hg diff -w". Such a feature might be a
>> nice addition to git.
> Thank you, that is very interesting. I just submitted a patch this morning that
> does exactly what you describe, but I called it "primer" instead of "defaults"
> because it seemed more explicit. Check it out.
I saw that. However, in Mercurial's case, the [defaults] section applies
to all commands (including new ones introduced by extensions). That is,
it looks like your patch adds a diff.primer, but Mercurial would add a
primer.* that allows you to have a primer.diff, primer.commit, etc.
For example, wouldn't it be nice to have something like...
[defaults]
diff = -w
commit = -a
etc.? Right now the only way you can do that is with aliases (e.g., I
have an alias from "ci" to "commit -a").
This task is pretty easy in Mercurial because Mercurial porcelains are
implemented as modules that are all executed through the central "hg"
command. This isn't the case with git.
(on a somewhat unrelated note, Mercurial does have a [diff] section that
allows for, say, forcing the use of git diff and other things)
--Ted
--
Ted Pavlic <ted@tedpavlic.com>
Please visit my ALS association page:
http://web.alsa.org/goto/tedpavlic
My family appreciates your support in the fight to defeat ALS.
^ permalink raw reply
* Re: [PATCH] gitweb: last-modified time should be commiter, not author
From: Jakub Narebski @ 2009-01-26 1:54 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <1232923370-4427-1-git-send-email-giuseppe.bilotta@gmail.com>
On Sun, 25 Jan 2009, Giuseppe Bilotta wrote:
> Subject: [PATCH] gitweb: last-modified time should be commiter, not author
Should be really either "[PATCH 1/2]" or "[PATCH 5/4]" or "[PATCH 5/6]"
just in case for the next patch, because next patch _depends_ on this
one, and just in case of threading problem it should be marked as it;
it also makes easier to apply patches from emails saved as individual
files each.
>
> The last-modified time header added by RSS to increase cache hits from
> readers should be set to the date the repository was last modified. The
> author time in this respect is not a good guess because the last commit
> might come from a oldish patch.
>
> Use the committer time for the last-modified header to ensure a more
> correct guess of the last time the repository was modified.
Good catch, good thinking IMHO. Committer date has much better chance
to be monotonic than author date, and is more close related to
_publishing_ date (author date is more of _creation_ date).
Lack signoff; if Junio forges it (or you reply that it should be
signed off), you can add from me
Acked-by: Jakub Narebski <jnareb@gmail.com>
P.S. I wonder what other web interfaces do, for example cgit. I guess
that web interfaces for other SCMs like SVN::Web, ViewVC etc. do not
have this problem because they have only one, single date.
> ---
> gitweb/gitweb.perl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 756868a..8c49c75 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6015,7 +6015,7 @@ sub git_feed {
> }
> if (defined($commitlist[0])) {
> %latest_commit = %{$commitlist[0]};
> - %latest_date = parse_date($latest_commit{'author_epoch'});
> + %latest_date = parse_date($latest_commit{'committer_epoch'});
> print $cgi->header(
> -type => $content_type,
> -charset => 'utf-8',
> --
> 1.5.6.5
>
>
--
Jakub Narebski
Poland
^ 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