* Re: [PATCH v15 13/27] bisect--helper: `bisect_start` shell function partially in C
From: Stephan Beyer @ 2016-11-20 20:19 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <52f6241f-e584-d830-ca66-084dc509c7fe@gmx.net>
On 11/20/2016 09:01 PM, Stephan Beyer wrote:
> First, replace the current set_terms() by
>
> static void set_terms(struct bisect_terms *terms, const char *bad,
> const char *good)
> {
> terms->term_good = xstrdup(good);
> terms->term_bad = xstrdup(bad);
> }
>
> ie, without calling write_terms(...).
I did not want to confuse you here but I forgot to mention that there
should also be freeing code, i.e. initialize your terms to NULL in the
beginning of cmd_builtin__helper, and always free them if it is not
null. This freeing code could also be in an extra function free_terms()
and you call it in set_terms() and for cleanup in the end.
^ permalink raw reply
* [PATCH] Remove dependency on deprecated Net::SMTP::SSL
From: Mike Fisher @ 2016-11-20 21:18 UTC (permalink / raw)
To: git
Refactor send_message() to remove dependency on deprecated
Net::SMTP::SSL:
<http://search.cpan.org/~rjbs/Net-SMTP-SSL-1.04/lib/Net/SMTP/SSL.pm#DEPRECATED>
Signed-off-by: Mike Fisher <mfisher@csh.rit.edu>
---
git-send-email.perl | 54
+++++++++++++++++++++++++----------------------------
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index da81be4..fc166c5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1330,15 +1330,17 @@ Message-Id: $message_id
print $sm "$header\n$message";
close $sm or die $!;
} else {
-
if (!defined $smtp_server) {
die "The required SMTP server is not properly defined."
}
+ require Net::SMTP;
+ $smtp_domain ||= maildomain();
+ my $smtp_ssl = 0;
+
if ($smtp_encryption eq 'ssl') {
$smtp_server_port ||= 465; # ssmtp
- require Net::SMTP::SSL;
- $smtp_domain ||= maildomain();
+ $smtp_ssl = 1;
require IO::Socket::SSL;
# Suppress "variable accessed once" warning.
@@ -1347,37 +1349,31 @@ Message-Id: $message_id
$IO::Socket::SSL::DEBUG = 1;
}
- # Net::SMTP::SSL->new() does not forward any SSL options
IO::Socket::SSL::set_client_defaults(
ssl_verify_params());
- $smtp ||= Net::SMTP::SSL->new($smtp_server,
- Hello => $smtp_domain,
- Port => $smtp_server_port,
- Debug => $debug_net_smtp);
}
else {
- require Net::SMTP;
- $smtp_domain ||= maildomain();
$smtp_server_port ||= 25;
- $smtp ||= Net::SMTP->new($smtp_server,
- Hello => $smtp_domain,
- Debug => $debug_net_smtp,
- Port => $smtp_server_port);
- if ($smtp_encryption eq 'tls' && $smtp) {
- require Net::SMTP::SSL;
- $smtp->command('STARTTLS');
- $smtp->response();
- if ($smtp->code == 220) {
- $smtp = Net::SMTP::SSL->start_SSL($smtp,
- ssl_verify_params())
- or die "STARTTLS failed! ".IO::Socket::SSL::errstr();
- $smtp_encryption = '';
- # Send EHLO again to receive fresh
- # supported commands
- $smtp->hello($smtp_domain);
- } else {
- die "Server does not support STARTTLS! ".$smtp->message;
- }
+ }
+
+ $smtp ||= Net::SMTP->new($smtp_server,
+ Hello => $smtp_domain,
+ Port => $smtp_server_port,
+ Debug => $debug_net_smtp,
+ SSL => $smtp_ssl);
+
+ if ($smtp_encryption eq 'tls' && $smtp) {
+ $smtp->command('STARTTLS');
+ $smtp->response();
+ if ($smtp->code == 220) {
+ $smtp->starttls(ssl_verify_params())
+ or die "STARTTLS failed! ".IO::Socket::SSL::errstr();
+ $smtp_encryption = '';
+ # Send EHLO again to receive fresh
+ # supported commands
+ $smtp->hello($smtp_domain);
+ } else {
+ die "Server does not support STARTTLS! ".$smtp->message;
}
}
--
2.9.3 (Apple Git-75)
^ permalink raw reply related
* Re: [PATCH] Remove dependency on deprecated Net::SMTP::SSL
From: brian m. carlson @ 2016-11-20 21:53 UTC (permalink / raw)
To: Mike Fisher; +Cc: git
In-Reply-To: <451E4A46-BA43-41A5-9E68-DE0D89BE676A@csh.rit.edu>
[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]
On Sun, Nov 20, 2016 at 04:18:16PM -0500, Mike Fisher wrote:
> Refactor send_message() to remove dependency on deprecated
> Net::SMTP::SSL:
>
> <http://search.cpan.org/~rjbs/Net-SMTP-SSL-1.04/lib/Net/SMTP/SSL.pm#DEPRECATED>
As much as I hate to say this, I think this is going to cause
compatibility problems. Net::SMTP is part of core Perl (as of v5.7.3),
but the version you want to rely on (which you did not provide an
explicit dependency on) is from October 2014.
That basically means that no Perl on a Red Hat or CentOS system is going
to provide that support, since RHEL 7 was released in June 2014.
Providing an updated Git on those platforms would require replacing the
system Perl or parts of it, which would be undesirable. This would
affect Debian 7 as well.
We currently support Perl 5.8 [0], so if you want to remove support for
Net::SMTP::SSL, I'd recommend a solution that works with that version.
[0] I personally believe we should drop support for Perl older than
5.10.1 (if not newer), but that's my opinion and it isn't shared by
other list regulars.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [PATCH] Remove dependency on deprecated Net::SMTP::SSL
From: Torsten Bögershausen @ 2016-11-21 5:37 UTC (permalink / raw)
To: Mike Fisher, git
In-Reply-To: <451E4A46-BA43-41A5-9E68-DE0D89BE676A@csh.rit.edu>
>On 20/11/16 22:18, Mike Fisher wrote:
Thanks for contributing to Git.
One comment on the head line:
>Refactor send_message() to remove dependency on deprecated
Net::SMTP::SSL
The word "refactor" may be used in other way: Re-structure the code,
and use the same API.
"Remove dependency on deprecated Net::SMTP::SSL"
> Refactor send_message() to remove dependency on deprecated
> Net::SMTP::SSL:
Is there a security risk with require Net::SMTP::SSL ?
If yes, the commit message should state this.
If no:
Even if it is deprecated, is it still in use somewhere ?
Does it hurt someone, is there any OS release where the old code doesn't work
anymore ?
Or is it "only" nice to have ?
Since when does Net::SMTP include Net::SMTP::SSL ?
On which system has the change been tested ?
I think the commit message could and should give more information like this.
My comments may be over-critical.
Lets see if other people from the list know more than me.
>
> <http://search.cpan.org/~rjbs/Net-SMTP-SSL-1.04/lib/Net/SMTP/SSL.pm#DEPRECATED>
>
> Signed-off-by: Mike Fisher <mfisher@csh.rit.edu>
> ---
> git-send-email.perl | 54 +++++++++++++++++++++++++----------------------------
> 1 file changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index da81be4..fc166c5 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1330,15 +1330,17 @@ Message-Id: $message_id
> print $sm "$header\n$message";
> close $sm or die $!;
> } else {
> -
I can see one refactoring, that is the removal of an empty line.
>
> if (!defined $smtp_server) {
> die "The required SMTP server is not properly defined."
> }
>
> + require Net::SMTP;
> + $smtp_domain ||= maildomain();
> + my $smtp_ssl = 0;
> +
> if ($smtp_encryption eq 'ssl') {
> $smtp_server_port ||= 465; # ssmtp
> - require Net::SMTP::SSL;
> - $smtp_domain ||= maildomain();
> + $smtp_ssl = 1;
> require IO::Socket::SSL;
>
> # Suppress "variable accessed once" warning.
> @@ -1347,37 +1349,31 @@ Message-Id: $message_id
> $IO::Socket::SSL::DEBUG = 1;
> }
>
> - # Net::SMTP::SSL->new() does not forward any SSL options
> IO::Socket::SSL::set_client_defaults(
> ssl_verify_params());
> - $smtp ||= Net::SMTP::SSL->new($smtp_server,
> - Hello => $smtp_domain,
> - Port => $smtp_server_port,
> - Debug => $debug_net_smtp);
> }
> else {
> - require Net::SMTP;
> - $smtp_domain ||= maildomain();
> $smtp_server_port ||= 25;
> - $smtp ||= Net::SMTP->new($smtp_server,
> - Hello => $smtp_domain,
> - Debug => $debug_net_smtp,
> - Port => $smtp_server_port);
> - if ($smtp_encryption eq 'tls' && $smtp) {
> - require Net::SMTP::SSL;
> - $smtp->command('STARTTLS');
> - $smtp->response();
> - if ($smtp->code == 220) {
> - $smtp = Net::SMTP::SSL->start_SSL($smtp,
> - ssl_verify_params())
> - or die "STARTTLS failed! ".IO::Socket::SSL::errstr();
> - $smtp_encryption = '';
> - # Send EHLO again to receive fresh
> - # supported commands
> - $smtp->hello($smtp_domain);
> - } else {
> - die "Server does not support STARTTLS! ".$smtp->message;
> - }
> + }
> +
> + $smtp ||= Net::SMTP->new($smtp_server,
> + Hello => $smtp_domain,
> + Port => $smtp_server_port,
> + Debug => $debug_net_smtp,
> + SSL => $smtp_ssl);
> +
> + if ($smtp_encryption eq 'tls' && $smtp) {
> + $smtp->command('STARTTLS');
> + $smtp->response();
> + if ($smtp->code == 220) {
> + $smtp->starttls(ssl_verify_params())
> + or die "STARTTLS failed! ".IO::Socket::SSL::errstr();
> + $smtp_encryption = '';
> + # Send EHLO again to receive fresh
> + # supported commands
> + $smtp->hello($smtp_domain);
> + } else {
> + die "Server does not support STARTTLS! ".$smtp->message;
> }
> }
>
^ permalink raw reply
* Feature request: Improve diff algorithm
From: KES @ 2016-11-21 8:11 UTC (permalink / raw)
To: git
Hi.
I have some question about how diff works then give proposal:
it will be very useful for each "symbol" store additional meta info as source line length. So in this case when git counter two equal sequence of commands it will do further comparison: Adds 23 chars deletes none VS adds 75 chars deletes 46
Actually I got this:
@@ -129,8 +132,9 @@ sub _preprocess_message {
sub _process_message {
my ($self, $message) = @_;
- my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
+ my $time = [ gettimeofday ];
+ my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
return $self->send_error(ERROR_REQUEST_INVALID)
unless defined($method);
Instead of expected:
@@ -129,6 +132,8 @@ sub _preprocess_message {
sub _process_message {
my ($self, $message) = @_;
+ my $time = [ gettimeofday ];
+
my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
-
return $self->send_error(ERROR_REQUEST_INVALID)
Details: http://stackoverflow.com/questions/40550751/unexpected-result-in-git-diff/40552165?noredirect=1#comment68648377_40552165
^ permalink raw reply
* Re: [PATCH v7 14/17] ref-filter: allow porcelain to translate messages in the output
From: Matthieu Moy @ 2016-11-21 8:41 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Jakub Narębski, Git List, Jacob Keller
In-Reply-To: <CAOLa=ZRU--FyFWNvv7RGH2mho36hvUu2ye=JOapXZ6QoEy=nRw@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> cc'in Matthieu since he wrote the patch.
>
> On Sat, Nov 19, 2016 at 4:16 AM, Jakub Narębski <jnareb@gmail.com> wrote:
>> W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
>>> From: Karthik Nayak <karthik.188@gmail.com>
>>>
>>> Introduce setup_ref_filter_porcelain_msg() so that the messages used in
>>> the atom %(upstream:track) can be translated if needed. This is needed
>>> as we port branch.c to use ref-filter's printing API's.
>>>
>>> Written-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
>>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>>> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
>>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>>> ---
>>> ref-filter.c | 28 ++++++++++++++++++++++++----
>>> ref-filter.h | 2 ++
>>> 2 files changed, 26 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ref-filter.c b/ref-filter.c
>>> index b47b900..944671a 100644
>>> --- a/ref-filter.c
>>> +++ b/ref-filter.c
>>> @@ -15,6 +15,26 @@
>>> #include "version.h"
>>> #include "wt-status.h"
>>>
>>> +static struct ref_msg {
>>> + const char *gone;
>>> + const char *ahead;
>>> + const char *behind;
>>> + const char *ahead_behind;
>>> +} msgs = {
>>> + "gone",
>>> + "ahead %d",
>>> + "behind %d",
>>> + "ahead %d, behind %d"
>>> +};
>>> +
>>> +void setup_ref_filter_porcelain_msg(void)
>>> +{
>>> + msgs.gone = _("gone");
>>> + msgs.ahead = _("ahead %d");
>>> + msgs.behind = _("behind %d");
>>> + msgs.ahead_behind = _("ahead %d, behind %d");
>>> +}
>>
>> Do I understand it correctly that this mechanism is here to avoid
>> repeated calls into gettext, as those messages would get repeated
>> over and over; otherwise one would use foo = N_("...") and _(foo),
>> isn't it?
That's not the primary goal. The primary goal is to keep untranslated,
and immutable messages in plumbing commands. We may decide one day that
_("gone") is not the best message for the end user and replace it with,
say, _("vanished"), but the "gone" has to remain the same forever and
regardless of the user's config for scripts using it.
We could have written
in_porcelain ? _("gone") : "gone"
here and there in the code, but having a single place where we set all
the messages seems simpler. Call setup_ref_filter_porcelain_msg() and
get the porcelain messages, don't call it and keep the plumbing
messages.
Note that it's not the first place in the code where we do this, see
setup_unpack_trees_porcelain in unpack-trees.c for another instance.
Karthik: the commit message could be improved, for example:
Introduce setup_ref_filter_porcelain_msg() so that the messages used in
the atom %(upstream:track) can be translated if needed. By default, keep
the messages untranslated, which is the right behavior for plumbing
commands. This is needed as we port branch.c to use ref-filter's
printing API's.
Why not a comment right below "} msgs = {" saying e.g.:
/* Untranslated plumbing messages: */
>> I wonder if there is some way to avoid duplication here, but I don't
>> see anything easy and safe (e.g. against running setup_*() twice).
I think we do need duplication for the reason above.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* --root seems to cancel out --keep-empty in git-rebase
From: Pierre Ossman @ 2016-11-21 13:30 UTC (permalink / raw)
To: git
As the subject says, these two commands give identical behaviour:
git rebase --interactive --keep-empty --root
and:
git rebase --interactive --root
I.e. --keep-empty has no effect when --root is specified.
Ideas for workaround welcome.
Regards
--
Pierre Ossman Software Development
Cendio AB https://cendio.com
Teknikringen 8 https://twitter.com/ThinLinc
583 30 Linköping https://facebook.com/ThinLinc
Phone: +46-13-214600 https://plus.google.com/+CendioThinLinc
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* [PATCH 0/3] Fix problems with rebase -i when core.commentchar is defined
From: Johannes Schindelin @ 2016-11-21 14:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
The Git for Windows project recently got a bug report that detailed how
`git rebase -i` fails when core.commentchar=auto:
https://groups.google.com/forum/#!topic/git-for-windows/eOZKjkgyX1Q
This patch series fixes rebase -i's handling of core.commentchar.
Johannes Schindelin (3):
rebase -i: identify problems with core.commentchar
stripspace: respect repository config
rebase -i: handle core.commentChar=auto
builtin/stripspace.c | 4 +++-
git-rebase--interactive.sh | 13 +++++++++++--
t/t0030-stripspace.sh | 7 +++++++
t/t3404-rebase-interactive.sh | 12 ++++++++++++
4 files changed, 33 insertions(+), 3 deletions(-)
base-commit: 1310affe024fba407bff55dbe65cd6d670c8a32d
Published-As: https://github.com/dscho/git/releases/tag/rebase-i-commentchar-v1
Fetch-It-Via: git fetch https://github.com/dscho/git rebase-i-commentchar-v1
--
2.10.1.583.g721a9e0
^ permalink raw reply
* [PATCH 2/3] stripspace: respect repository config
From: Johannes Schindelin @ 2016-11-21 14:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <cover.1479737858.git.johannes.schindelin@gmx.de>
When eff80a9 (Allow custom "comment char", 2013-01-16) taught the
`stripspace` command to respect the config setting `core.commentChar`,
it forgot that this variable may be defined in .git/config.
So when rebasing interactively with a commentChar defined in the current
repository's config, the help text at the bottom of the edit script
potentially used an incorrect comment character. This was not only
funny-looking, but also resulted in tons of warnings like this one:
Warning: the command isn't recognized in the following line
- #
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/stripspace.c | 4 +++-
t/t0030-stripspace.sh | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 15e716e..1e62a00 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -44,8 +44,10 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
if (argc)
usage_with_options(stripspace_usage, options);
- if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
+ if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
+ setup_git_directory_gently(NULL);
git_config(git_default_config, NULL);
+ }
if (strbuf_read(&buf, 0, 1024) < 0)
die_errno("could not read the input");
diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index 202ac07..67f77df 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -432,7 +432,7 @@ test_expect_success '-c with changed comment char' '
test_cmp expect actual
'
-test_expect_failure '-c with comment char defined in .git/config' '
+test_expect_success '-c with comment char defined in .git/config' '
test_config core.commentchar = &&
printf "= foo\n" >expect &&
printf "foo" | git stripspace -c >actual &&
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Johannes Schindelin @ 2016-11-21 14:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <cover.1479737858.git.johannes.schindelin@gmx.de>
The interactive rebase does not currently play well with
core.commentchar. Let's add some tests to identify those problems.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t0030-stripspace.sh | 7 +++++++
t/t3404-rebase-interactive.sh | 12 ++++++++++++
2 files changed, 19 insertions(+)
diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index 29e91d8..202ac07 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -432,6 +432,13 @@ test_expect_success '-c with changed comment char' '
test_cmp expect actual
'
+test_expect_failure '-c with comment char defined in .git/config' '
+ test_config core.commentchar = &&
+ printf "= foo\n" >expect &&
+ printf "foo" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'avoid SP-HT sequence in commented line' '
printf "#\tone\n#\n# two\n" >expect &&
printf "\tone\n\ntwo\n" | git stripspace -c >actual &&
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index e38e296..e080399 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -976,6 +976,18 @@ test_expect_success 'rebase -i respects core.commentchar' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
+test_expect_failure 'rebase -i respects core.commentchar=auto' '
+ test_config core.commentchar auto &&
+ write_script copy-edit-script.sh <<-\EOF &&
+ cp "$1" edit-script
+ EOF
+ test_set_editor "$(pwd)/copy-edit-script.sh" &&
+ test_when_finished "git rebase --abort || :" &&
+ git rebase -i HEAD^ &&
+ grep "^#" edit-script &&
+ test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)"
+'
+
test_expect_success 'rebase -i, with <onto> and <upstream> specified as :/quuxery' '
test_when_finished "git branch -D torebase" &&
git checkout -b torebase branch1 &&
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Johannes Schindelin @ 2016-11-21 14:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <cover.1479737858.git.johannes.schindelin@gmx.de>
When 84c9dc2 (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) extended the core.commentChar functionality to
allow for the value 'auto', it forgot that rebase -i was already taught to
handle core.commentChar, and in turn forgot to let rebase -i handle that
new value gracefully.
Reported by Taufiq Hoven.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
git-rebase--interactive.sh | 13 +++++++++++--
t/t3404-rebase-interactive.sh | 2 +-
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ca994c5..6bb740c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -93,8 +93,17 @@ eval '
GIT_CHERRY_PICK_HELP="$resolvemsg"
export GIT_CHERRY_PICK_HELP
-comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
-: ${comment_char:=#}
+comment_char=$(git config --get core.commentchar 2>/dev/null)
+case "$comment_char" in
+''|auto)
+ comment_char=#
+ ;;
+?)
+ ;;
+*)
+ comment_char=$(comment_char | cut -c1)
+ ;;
+esac
warn () {
printf '%s\n' "$*" >&2
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index e080399..0f3d177 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -976,7 +976,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
-test_expect_failure 'rebase -i respects core.commentchar=auto' '
+test_expect_success 'rebase -i respects core.commentchar=auto' '
test_config core.commentchar auto &&
write_script copy-edit-script.sh <<-\EOF &&
cp "$1" edit-script
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* Re: [PATCH] i18n: Fixed unmatched single quote in error message
From: Johannes Schindelin @ 2016-11-21 14:22 UTC (permalink / raw)
To: Jiang Xin; +Cc: Junio C Hamano, Git List
In-Reply-To: <721cdca4d000627fa163da15c515a50fd9529a62.1479644251.git.worldhello.net@gmail.com>
Hi,
On Sun, 20 Nov 2016, Jiang Xin wrote:
> Fixed unmatched single quote introduced by commit:
>
> * f56fffef9a sequencer: teach write_message() to append an optional LF
>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
ACK!
Thank you,
Dscho
^ permalink raw reply
* Re: Feature request: Improve diff algorithm
From: Jacob Keller @ 2016-11-21 16:56 UTC (permalink / raw)
To: KES; +Cc: git
In-Reply-To: <14657461479715884@web21h.yandex.ru>
On Mon, Nov 21, 2016 at 12:11 AM, KES <kes-kes@yandex.ru> wrote:
> Hi.
>
Hi,
> I have some question about how diff works then give proposal:
>
> it will be very useful for each "symbol" store additional meta info as source line length. So in this case when git counter two equal sequence of commands it will do further comparison: Adds 23 chars deletes none VS adds 75 chars deletes 46
>
> Actually I got this:
>
> @@ -129,8 +132,9 @@ sub _preprocess_message {
> sub _process_message {
> my ($self, $message) = @_;
>
> - my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
> + my $time = [ gettimeofday ];
>
> + my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
> return $self->send_error(ERROR_REQUEST_INVALID)
> unless defined($method);
>
> Instead of expected:
> @@ -129,6 +132,8 @@ sub _preprocess_message {
> sub _process_message {
> my ($self, $message) = @_;
>
> + my $time = [ gettimeofday ];
> +
> my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
> -
> return $self->send_error(ERROR_REQUEST_INVALID)
>
Have you tried the various options for git to search for smaller
diffs? Or using the other diff algorithms such as histogram instead of
patience?
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 0/3] Fix problems with rebase -i when core.commentchar is defined
From: Jacob Keller @ 2016-11-21 16:58 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Git mailing list, Junio C Hamano, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <cover.1479737858.git.johannes.schindelin@gmx.de>
On Mon, Nov 21, 2016 at 6:18 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> The Git for Windows project recently got a bug report that detailed how
> `git rebase -i` fails when core.commentchar=auto:
>
> https://groups.google.com/forum/#!topic/git-for-windows/eOZKjkgyX1Q
>
> This patch series fixes rebase -i's handling of core.commentchar.
>
>
> Johannes Schindelin (3):
> rebase -i: identify problems with core.commentchar
> stripspace: respect repository config
> rebase -i: handle core.commentChar=auto
>
> builtin/stripspace.c | 4 +++-
> git-rebase--interactive.sh | 13 +++++++++++--
> t/t0030-stripspace.sh | 7 +++++++
> t/t3404-rebase-interactive.sh | 12 ++++++++++++
> 4 files changed, 33 insertions(+), 3 deletions(-)
>
>
> base-commit: 1310affe024fba407bff55dbe65cd6d670c8a32d
> Published-As: https://github.com/dscho/git/releases/tag/rebase-i-commentchar-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git rebase-i-commentchar-v1
>
> --
> 2.10.1.583.g721a9e0
>
The series makes sense to me.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 0/2] add format specifiers to display trailers
From: Junio C Hamano @ 2016-11-21 17:23 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jacob Keller, Jonathan Tan, Git mailing list
In-Reply-To: <CA+P7+xrQEBYQQhqJQQCpLrs+4WOJOvH1X27w5Ou=2VPT=FegGQ@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
>> We have %s and %b so that we can reconstruct the whole thing by
>> using both. It is unclear how %bT fits in this picture. I wonder
>> if we also need another placeholder that expands to the body of the
>> message without the trailer---otherwise the whole set would become
>> incoherent, no?
>
> I'm not entirely sure what to do here. I just wanted a way to easily
> format "just the trailers" of a message. We could add something that
> formats just the non-trailers, that's not too difficult. Not really
> sure what I'd call it though.
I was wondering if %(log:<name of a part>) was a better way to go.
%(log:title) and %(log:body) would be equivalents of traditional %s
and %b, and %(log:body) in turn would be a shorter way to write
%(log:description)%+(log:trailer), i.e. show the message body, and
if there is a trailer block, add it after adding a blank line.
Or something like that?
^ permalink raw reply
* Re: [PATCH] i18n: Fixed unmatched single quote in error message
From: Junio C Hamano @ 2016-11-21 17:31 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jiang Xin, Git List
In-Reply-To: <alpine.DEB.2.20.1611211522280.3746@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi,
>
> On Sun, 20 Nov 2016, Jiang Xin wrote:
>
>> Fixed unmatched single quote introduced by commit:
>>
>> * f56fffef9a sequencer: teach write_message() to append an optional LF
>>
>> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
>
> ACK!
>
> Thank you,
> Dscho
Thanks, both.
^ permalink raw reply
* RE: [PATCH 13/16] submodule: teach unpack_trees() to update submodules
From: David Turner @ 2016-11-21 18:12 UTC (permalink / raw)
To: 'Stefan Beller'
Cc: git@vger.kernel.org, bmwill@google.com, gitster@pobox.com,
jrnieder@gmail.com, mogulguy10@gmail.com
In-Reply-To: <CAGZ79kb-TKgQm1_p3NyJS7mso2FFWMNHa7F8vwzocjSJfyjXJQ@mail.gmail.com>
> -----Original Message-----
> From: Stefan Beller [mailto:sbeller@google.com]
> >> + if (submodule_is_interesting(old->name,
> null_sha1)
> >> + && ok_to_remove_submodule(old->name))
> >> + return 0;
> >> + }
> >
> > Do we need a return 1 in here somewhere? Because otherwise, we fall
> through and return 0 later.
>
> Otherwise we would fall through and run
>
> if (errno == ENOENT)
> return 0;
> return o->gently ? -1 :
> add_rejected_path(o, error_type, ce->name);
>
> which produces different results than 0?
Oh, I see. I was misreading that errno check.
^ permalink raw reply
* Re: [PATCH v4 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Brandon Williams @ 2016-11-21 18:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <20161118225220.GA76630@google.com>
On 11/18, Brandon Williams wrote:
> Also, in order to use the tree_entry_interesting code it looks like I'll
> either have to pipe through a flag saying 'yes i want to match against
> submodules' like I did for the other pathspec codepath. Either that or
> add functionality to perform wildmatching against partial matches (ie
> directories and submodules) since currently the tree_entry_interesting
> code path just punts and says 'well say it matches for now and check
> again later' whenever it runs into a directory (I can't really make it
> do that for submodules without a flag of somesort as tests could break).
> Or maybe both?
Looks like my initial assumption was incorrect, I just needed to be
smarter than punting when running into a submodule. Should be able to
just ensure that the entry matches up to at least the first wildcard
character before punting and all should be good.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Junio C Hamano @ 2016-11-21 18:15 UTC (permalink / raw)
To: Johannes Schindelin, Jeff King
Cc: git, Ralf Thielow, Nguyễn Thái Ngọc Duy,
Taufiq Hoven
In-Reply-To: <f47dce15719954d6d4d8a550856757366871143e.1479737858.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 29e91d8..202ac07 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,6 +432,13 @@ test_expect_success '-c with changed comment char' '
> test_cmp expect actual
> '
>
> +test_expect_failure '-c with comment char defined in .git/config' '
> + test_config core.commentchar = &&
> + printf "= foo\n" >expect &&
> + printf "foo" | git stripspace -c >actual &&
We'd want "\n" on this printf to match the one before as well, as
this test is not about "does stripspace complete an incomplete
line?", I think.
I could amend it while queuing, but I need to know if I am missing a
reason why this must be an incomplete line before doing so.
> + test_cmp expect actual
> +'
> +
Is this a recent regression? When applied on top of 'maint' or
older, it seems to pass just fine.
... Goes and looks ...
Interesting. Peff's b9605bc4f2 ("config: only read .git/config from
configured repos", 2016-09-12) is where this starts failing, which
is understandable given the code change to builtin/stripspace.c in
[2/3].
The analysis of the log message in [2/3] is wrong and needs
updating, though. In the old world order it worked by accident to
call git_config() without calling setup_git_directory(); after
b9605bc4f2, that no longer is valid and is exposed as a bug.
Your [2/3] is a good fix for that change.
In any case, well spotted.
> test_expect_success 'avoid SP-HT sequence in commented line' '
> printf "#\tone\n#\n# two\n" >expect &&
> printf "\tone\n\ntwo\n" | git stripspace -c >actual &&
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index e38e296..e080399 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -976,6 +976,18 @@ test_expect_success 'rebase -i respects core.commentchar' '
> test B = $(git cat-file commit HEAD^ | sed -ne \$p)
> '
>
> +test_expect_failure 'rebase -i respects core.commentchar=auto' '
> + test_config core.commentchar auto &&
> + write_script copy-edit-script.sh <<-\EOF &&
> + cp "$1" edit-script
> + EOF
> + test_set_editor "$(pwd)/copy-edit-script.sh" &&
> + test_when_finished "git rebase --abort || :" &&
> + git rebase -i HEAD^ &&
> + grep "^#" edit-script &&
This was added for debugging that was forgotten?
> + test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)"
This says "There shouldn't be any line left once we remove
'#'-commented lines, empty lines and pick insns.". OK.
The correction in [3/3] seems good.
> +'
> +
> test_expect_success 'rebase -i, with <onto> and <upstream> specified as :/quuxery' '
> test_when_finished "git branch -D torebase" &&
> git checkout -b torebase branch1 &&
^ permalink raw reply
* Re: Feature request: Improve diff algorithm
From: Stefan Beller @ 2016-11-21 18:17 UTC (permalink / raw)
To: Jacob Keller; +Cc: KES, git
In-Reply-To: <CA+P7+xrKfqOb-3FhKxXdnF6g9PktNWNzY+ZnHJS=yuVo1YdXzg@mail.gmail.com>
On Mon, Nov 21, 2016 at 8:56 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Mon, Nov 21, 2016 at 12:11 AM, KES <kes-kes@yandex.ru> wrote:
>> Hi.
>>
>
> Hi,
>
>> I have some question about how diff works then give proposal:
>>
>> it will be very useful for each "symbol" store additional meta info as source line length. So in this case when git counter two equal sequence of commands it will do further comparison: Adds 23 chars deletes none VS adds 75 chars deletes 46
>>
>> Actually I got this:
>>
>> @@ -129,8 +132,9 @@ sub _preprocess_message {
>> sub _process_message {
>> my ($self, $message) = @_;
>>
>> - my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>> + my $time = [ gettimeofday ];
>>
>> + my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>> return $self->send_error(ERROR_REQUEST_INVALID)
>> unless defined($method);
>>
>> Instead of expected:
>> @@ -129,6 +132,8 @@ sub _preprocess_message {
>> sub _process_message {
>> my ($self, $message) = @_;
>>
>> + my $time = [ gettimeofday ];
>> +
>> my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>> -
>> return $self->send_error(ERROR_REQUEST_INVALID)
>>
>
> Have you tried the various options for git to search for smaller
> diffs? Or using the other diff algorithms such as histogram instead of
> patience?
>
The newest version of Git comes with a flag to move around the diff
better, based on the work at https://github.com/mhagger/diff-slider-tools
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Junio C Hamano @ 2016-11-21 18:24 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, git, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqqbmx8k8c0.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
>> +test_expect_failure '-c with comment char defined in .git/config' '
>> + test_config core.commentchar = &&
>> + printf "= foo\n" >expect &&
>> + printf "foo" | git stripspace -c >actual &&
>
> We'd want "\n" on this printf to match the one before as well, as
> this test is not about "does stripspace complete an incomplete
> line?", I think.
>
> I could amend it while queuing, but I need to know if I am missing a
> reason why this must be an incomplete line before doing so.
>
>> + test_cmp expect actual
>> +'
>> +
>
> Is this a recent regression? When applied on top of 'maint' or
> older, it seems to pass just fine.
I think we can force failure by running this test somewhere other
than the top level of the working tree. A set of proposed amends
incoming ...
^ permalink raw reply
* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Johannes Sixt @ 2016-11-21 18:26 UTC (permalink / raw)
To: Johannes Schindelin, git
Cc: Junio C Hamano, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <9ef529a4fbb60990a91d7bbfdd49c6d20d49e442.1479737858.git.johannes.schindelin@gmx.de>
Am 21.11.2016 um 15:18 schrieb Johannes Schindelin:
> -comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> -: ${comment_char:=#}
> +comment_char=$(git config --get core.commentchar 2>/dev/null)
> +case "$comment_char" in
> +''|auto)
> + comment_char=#
> + ;;
> +?)
> + ;;
> +*)
> + comment_char=$(comment_char | cut -c1)
comment_char is a command? Did you mean
comment_char=$(echo "$comment_char" | cut -c1)
It could be written without forking a process:
comment_char=${comment_char%${comment_char#?}}
(aka "remove from the end what remains after removing the first character")
-- Hannes
^ permalink raw reply
* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Junio C Hamano @ 2016-11-21 18:40 UTC (permalink / raw)
To: Johannes Sixt
Cc: Johannes Schindelin, git, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <975c0002-8afe-efa6-d967-72f2a0d21169@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> comment_char is a command? Did you mean
>
> comment_char=$(echo "$comment_char" | cut -c1)
;-)
> It could be written without forking a process:
>
> comment_char=${comment_char%${comment_char#?}}
>
> (aka "remove from the end what remains after removing the first character")
Hopefully nobody would include any glob metacharacters in there,
e.g. "core.commentchar='=*'", which would break that?
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Jeff King @ 2016-11-21 18:49 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, git, Ralf Thielow,
Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqqbmx8k8c0.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 21, 2016 at 10:15:43AM -0800, Junio C Hamano wrote:
> > + test_cmp expect actual
> > +'
> > +
>
> Is this a recent regression? When applied on top of 'maint' or
> older, it seems to pass just fine.
>
> ... Goes and looks ...
>
> Interesting. Peff's b9605bc4f2 ("config: only read .git/config from
> configured repos", 2016-09-12) is where this starts failing, which
> is understandable given the code change to builtin/stripspace.c in
> [2/3].
>
> The analysis of the log message in [2/3] is wrong and needs
> updating, though. In the old world order it worked by accident to
> call git_config() without calling setup_git_directory(); after
> b9605bc4f2, that no longer is valid and is exposed as a bug.
Yeah, I noticed that while reading the patch. My b9605bc4f2 did regress
this case, but called out the fact that "cd subdir && git stripspace"
would never have worked. So one step back, 2 steps forward, and Dscho's
patch is the right step forward.
-Peff
^ permalink raw reply
* Re: Feature request: Improve diff algorithm
From: Jacob Keller @ 2016-11-21 18:55 UTC (permalink / raw)
To: Stefan Beller; +Cc: KES, git
In-Reply-To: <CAGZ79kZbGPneUXVEqJYhQAn+dfYve7qCjhO7QFaV1JBs3HD1aA@mail.gmail.com>
On Mon, Nov 21, 2016 at 10:17 AM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Nov 21, 2016 at 8:56 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Mon, Nov 21, 2016 at 12:11 AM, KES <kes-kes@yandex.ru> wrote:
>>> Hi.
>>>
>>
>> Hi,
>>
>>> I have some question about how diff works then give proposal:
>>>
>>> it will be very useful for each "symbol" store additional meta info as source line length. So in this case when git counter two equal sequence of commands it will do further comparison: Adds 23 chars deletes none VS adds 75 chars deletes 46
>>>
>>> Actually I got this:
>>>
>>> @@ -129,8 +132,9 @@ sub _preprocess_message {
>>> sub _process_message {
>>> my ($self, $message) = @_;
>>>
>>> - my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>>> + my $time = [ gettimeofday ];
>>>
>>> + my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>>> return $self->send_error(ERROR_REQUEST_INVALID)
>>> unless defined($method);
>>>
>>> Instead of expected:
>>> @@ -129,6 +132,8 @@ sub _preprocess_message {
>>> sub _process_message {
>>> my ($self, $message) = @_;
>>>
>>> + my $time = [ gettimeofday ];
>>> +
>>> my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>>> -
>>> return $self->send_error(ERROR_REQUEST_INVALID)
>>>
>>
>> Have you tried the various options for git to search for smaller
>> diffs? Or using the other diff algorithms such as histogram instead of
>> patience?
>>
>
> The newest version of Git comes with a flag to move around the diff
> better, based on the work at https://github.com/mhagger/diff-slider-tools
Unfortunately in this case, I'm not convinced that it will improve the
diff. It's worth a try as well though.
Thanks,
Jake
^ 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