git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/1] DiffHighlight.pm: Use correct /dev/null for UNIX and Windows
@ 2018-10-30 18:26 Chris. Webster via GitGitGadget
  2018-10-30 18:26 ` [PATCH 1/1] " chris via GitGitGadget
  2018-10-31 22:58 ` [PATCH v2 0/1] DiffHighlight.pm: " Chris. Webster via GitGitGadget
  0 siblings, 2 replies; 18+ messages in thread
From: Chris. Webster via GitGitGadget @ 2018-10-30 18:26 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Use File::Spec->devnull() for output redirection to avoid messages when
Windows version of Perl is first in path. The message 'The system cannot
find the path specified.' is displayed each time git is run to get colors.

chris (1):
  Use correct /dev/null for UNIX and Windows

 contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


base-commit: c670b1f876521c9f7cd40184bf7ed05aad843433
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-59%2Fwebstech%2Fmaster-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-59/webstech/master-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/59
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-30 18:26 [PATCH 0/1] DiffHighlight.pm: Use correct /dev/null for UNIX and Windows Chris. Webster via GitGitGadget
@ 2018-10-30 18:26 ` chris via GitGitGadget
  2018-10-31  3:56   ` Jeff King
                     ` (2 more replies)
  2018-10-31 22:58 ` [PATCH v2 0/1] DiffHighlight.pm: " Chris. Webster via GitGitGadget
  1 sibling, 3 replies; 18+ messages in thread
From: chris via GitGitGadget @ 2018-10-30 18:26 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, chris

From: chris <chris@webstech.net>

Use File::Spec->devnull() for output redirection to avoid messages
when Windows version of Perl is first in path.  The message 'The
system cannot find the path specified.' is displayed each time git is
run to get colors.

Signed-off-by: Chris. Webster <chris@webstech.net>
---
 contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index 536754583..7440aa1c4 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -4,6 +4,11 @@ use 5.008;
 use warnings FATAL => 'all';
 use strict;
 
+# Use the correct value for both UNIX and Windows (/dev/null vs nul)
+use File::Spec;
+
+my $NULL = File::Spec->devnull();
+
 # Highlight by reversing foreground and background. You could do
 # other things like bold or underline if you prefer.
 my @OLD_HIGHLIGHT = (
@@ -134,7 +139,7 @@ sub highlight_stdin {
 # fallback, which means we will work even if git can't be run.
 sub color_config {
 	my ($key, $default) = @_;
-	my $s = `git config --get-color $key 2>/dev/null`;
+	my $s = `git config --get-color $key 2>$NULL`;
 	return length($s) ? $s : $default;
 }
 
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-30 18:26 ` [PATCH 1/1] " chris via GitGitGadget
@ 2018-10-31  3:56   ` Jeff King
  2018-10-31  4:48     ` Junio C Hamano
  2018-10-31  4:54   ` Junio C Hamano
  2018-10-31  5:07   ` Junio C Hamano
  2 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2018-10-31  3:56 UTC (permalink / raw)
  To: chris via GitGitGadget; +Cc: git, Junio C Hamano, chris

On Tue, Oct 30, 2018 at 11:26:36AM -0700, chris via GitGitGadget wrote:

> From: chris <chris@webstech.net>

You might want to adjust your user.name. :)

> Use File::Spec->devnull() for output redirection to avoid messages
> when Windows version of Perl is first in path.  The message 'The
> system cannot find the path specified.' is displayed each time git is
> run to get colors.

Thanks, makes sense, and the patch looks good to me.

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-31  3:56   ` Jeff King
@ 2018-10-31  4:48     ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2018-10-31  4:48 UTC (permalink / raw)
  To: Jeff King; +Cc: chris via GitGitGadget, git, chris

Jeff King <peff@peff.net> writes:

> On Tue, Oct 30, 2018 at 11:26:36AM -0700, chris via GitGitGadget wrote:
>
>> From: chris <chris@webstech.net>
>
> You might want to adjust your user.name. :)

Yes, absolutely.  We'd want to see that the From: line and one of
the Signed-off-by: lines are idential.

>> Use File::Spec->devnull() for output redirection to avoid messages
>> when Windows version of Perl is first in path.  The message 'The
>> system cannot find the path specified.' is displayed each time git is
>> run to get colors.
>
> Thanks, makes sense, and the patch looks good to me.

Yup, and we already use File::Spec everywhere anyway, so this is not
a new dependency, either.  Which is very good.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-30 18:26 ` [PATCH 1/1] " chris via GitGitGadget
  2018-10-31  3:56   ` Jeff King
@ 2018-10-31  4:54   ` Junio C Hamano
       [not found]     ` <CAGT1KpWoGD0xgTrC-+X1WqY_M=2arYbs4ZX6Nnj-zHK6mgu+nw@mail.gmail.com>
  2018-10-31  5:07   ` Junio C Hamano
  2 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2018-10-31  4:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: chris via GitGitGadget, git, chris

"chris via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: chris <chris@webstech.net>
>
> Use File::Spec->devnull() for output redirection to avoid messages
> when Windows version of Perl is first in path.  The message 'The

Dscho, "Windows version of Perl is first in path" somehow feels
contradicting with what one of the topics I saw from you were trying
to enforce (or, at least, "set as the supported configuration").

I am guessing that the Perl you are building and shipping with Git
for Windows would yield what the shell that ends up running the
scriptlet `git config --get-color $key` prefers when asked for
File::Spec->devnull(), and nothing will break with this patch even
if that is "/dev/null", but I thought I'd double check.

Thanks.

> system cannot find the path specified.' is displayed each time git is
> run to get colors.
>
> Signed-off-by: Chris. Webster <chris@webstech.net>
> ---
>  contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
> index 536754583..7440aa1c4 100644
> --- a/contrib/diff-highlight/DiffHighlight.pm
> +++ b/contrib/diff-highlight/DiffHighlight.pm
> @@ -4,6 +4,11 @@ use 5.008;
>  use warnings FATAL => 'all';
>  use strict;
>  
> +# Use the correct value for both UNIX and Windows (/dev/null vs nul)
> +use File::Spec;
> +
> +my $NULL = File::Spec->devnull();
> +
>  # Highlight by reversing foreground and background. You could do
>  # other things like bold or underline if you prefer.
>  my @OLD_HIGHLIGHT = (
> @@ -134,7 +139,7 @@ sub highlight_stdin {
>  # fallback, which means we will work even if git can't be run.
>  sub color_config {
>  	my ($key, $default) = @_;
> -	my $s = `git config --get-color $key 2>/dev/null`;
> +	my $s = `git config --get-color $key 2>$NULL`;
>  	return length($s) ? $s : $default;
>  }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-30 18:26 ` [PATCH 1/1] " chris via GitGitGadget
  2018-10-31  3:56   ` Jeff King
  2018-10-31  4:54   ` Junio C Hamano
@ 2018-10-31  5:07   ` Junio C Hamano
  2018-10-31 11:14     ` Johannes Schindelin
  2 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2018-10-31  5:07 UTC (permalink / raw)
  To: chris via GitGitGadget; +Cc: git, chris, Johannes Schindelin

> Subject: Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows

As this is only about contrib/diff-highlight, please make it clear
that it is the area the patch affects on its title, i.e.

	Subject: diff-highlight: use File::Spec->devnull(), not /dev/null

or something like that.

> From: chris <chris@webstech.net>

Please make this line read like

	From: Chris Webster <chris@webstech.net>

i.e. the author should be the person who is signing off that patch.

> Use File::Spec->devnull() for output redirection to avoid messages
> when Windows version of Perl is first in path.  The message 'The
> system cannot find the path specified.' is displayed each time git is
> run to get colors.
>
> Signed-off-by: Chris. Webster <chris@webstech.net>
> ---
>  contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

There are a handful more instances of /dev/null found if you do

	$ git grep /dev/null -- \*.pl \*.pm

The one in perl/Git.pm must be shared by scripts written in Perl, so
it may be worth giving the same tweak to it, like this patch does to
the highlight script.

> diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
> index 536754583..7440aa1c4 100644
> --- a/contrib/diff-highlight/DiffHighlight.pm
> +++ b/contrib/diff-highlight/DiffHighlight.pm
> @@ -4,6 +4,11 @@ use 5.008;
>  use warnings FATAL => 'all';
>  use strict;
>  
> +# Use the correct value for both UNIX and Windows (/dev/null vs nul)
> +use File::Spec;
> +
> +my $NULL = File::Spec->devnull();
> +
>  # Highlight by reversing foreground and background. You could do
>  # other things like bold or underline if you prefer.
>  my @OLD_HIGHLIGHT = (
> @@ -134,7 +139,7 @@ sub highlight_stdin {
>  # fallback, which means we will work even if git can't be run.
>  sub color_config {
>  	my ($key, $default) = @_;
> -	my $s = `git config --get-color $key 2>/dev/null`;
> +	my $s = `git config --get-color $key 2>$NULL`;
>  	return length($s) ? $s : $default;
>  }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
       [not found]     ` <CAGT1KpWoGD0xgTrC-+X1WqY_M=2arYbs4ZX6Nnj-zHK6mgu+nw@mail.gmail.com>
@ 2018-10-31  5:41       ` Chris Webster
  2018-10-31  6:08         ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Chris Webster @ 2018-10-31  5:41 UTC (permalink / raw)
  To: gitster; +Cc: johannes.schindelin, gitgitgadget, git

Resending in text mode.

On Tue, Oct 30, 2018 at 10:20 PM Chris Webster <chris@webstech.net> wrote:
>
> On Tue, Oct 30, 2018 at 9:54 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "chris via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > From: chris <chris@webstech.net>
>> >
>> > Use File::Spec->devnull() for output redirection to avoid messages
>> > when Windows version of Perl is first in path.  The message 'The
>>
>> Dscho, "Windows version of Perl is first in path" somehow feels
>> contradicting with what one of the topics I saw from you were trying
>> to enforce (or, at least, "set as the supported configuration").
>>
>> I am guessing that the Perl you are building and shipping with Git
>> for Windows would yield what the shell that ends up running the
>> scriptlet `git config --get-color $key` prefers when asked for
>> File::Spec->devnull(), and nothing will break with this patch even
>> if that is "/dev/null", but I thought I'd double check.
>>
>> Thanks.
>>
This problem originally showed up in the
https://github.com/so-fancy/diff-so-fancy project, which has a copy of
DiffHighlight.pm.   That project allows diffsofancy (perl) to be run
from the command line without requiring the bash environment ((well ,
sort of) including the associated perl).
>
>> > system cannot find the path specified.' is displayed each time git is
>> > run to get colors.
>> >
>> > Signed-off-by: Chris. Webster <chris@webstech.net>
>> > ---
>> >  contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
>> >  1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
>> > index 536754583..7440aa1c4 100644
>> > --- a/contrib/diff-highlight/DiffHighlight.pm
>> > +++ b/contrib/diff-highlight/DiffHighlight.pm
>> > @@ -4,6 +4,11 @@ use 5.008;
>> >  use warnings FATAL => 'all';
>> >  use strict;
>> >
>> > +# Use the correct value for both UNIX and Windows (/dev/null vs nul)
>> > +use File::Spec;
>> > +
>> > +my $NULL = File::Spec->devnull();
>> > +
>> >  # Highlight by reversing foreground and background. You could do
>> >  # other things like bold or underline if you prefer.
>> >  my @OLD_HIGHLIGHT = (
>> > @@ -134,7 +139,7 @@ sub highlight_stdin {
>> >  # fallback, which means we will work even if git can't be run.
>> >  sub color_config {
>> >       my ($key, $default) = @_;
>> > -     my $s = `git config --get-color $key 2>/dev/null`;
>> > +     my $s = `git config --get-color $key 2>$NULL`;
>> >       return length($s) ? $s : $default;
>> >  }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-31  5:41       ` Chris Webster
@ 2018-10-31  6:08         ` Junio C Hamano
  2018-10-31 11:10           ` Johannes Schindelin
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2018-10-31  6:08 UTC (permalink / raw)
  To: Chris Webster; +Cc: johannes.schindelin, gitgitgadget, git

Chris Webster <chris@webstech.net> writes:

>>> > Use File::Spec->devnull() for output redirection to avoid messages
>>> > when Windows version of Perl is first in path.  The message 'The
>>>
>>> Dscho, "Windows version of Perl is first in path" somehow feels
>>> contradicting with what one of the topics I saw from you were trying
>>> to enforce (or, at least, "set as the supported configuration").
>>>
>>> I am guessing that the Perl you are building and shipping with Git
>>> for Windows would yield what the shell that ends up running the
>>> scriptlet `git config --get-color $key` prefers when asked for
>>> File::Spec->devnull(), and nothing will break with this patch even
>>> if that is "/dev/null", but I thought I'd double check.
>>>
>>> Thanks.
>>>
> This problem originally showed up in the
> https://github.com/so-fancy/diff-so-fancy project, which has a copy of
> DiffHighlight.pm.   That project allows diffsofancy (perl) to be run
> from the command line without requiring the bash environment ((well ,
> sort of) including the associated perl).

Thanks for additional comments.  

In any case, Windows is not my bailiwick, so I'll hope that the
above comments from you would help Dscho in his response and wait.
I know use of File::Spec->devnull() won't hurt POSIX folks so making
sure this won't break Git for Windows is the primary thing I woudl
worry about this patch.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-31  6:08         ` Junio C Hamano
@ 2018-10-31 11:10           ` Johannes Schindelin
  2018-11-01  1:57             ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2018-10-31 11:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Chris Webster, gitgitgadget, git

Hi Junio,

On Wed, 31 Oct 2018, Junio C Hamano wrote:

> Chris Webster <chris@webstech.net> writes:
> 
> >>> > Use File::Spec->devnull() for output redirection to avoid messages
> >>> > when Windows version of Perl is first in path.  The message 'The
> >>>
> >>> Dscho, "Windows version of Perl is first in path" somehow feels
> >>> contradicting with what one of the topics I saw from you were trying
> >>> to enforce (or, at least, "set as the supported configuration").
> >>>
> >>> I am guessing that the Perl you are building and shipping with Git
> >>> for Windows would yield what the shell that ends up running the
> >>> scriptlet `git config --get-color $key` prefers when asked for
> >>> File::Spec->devnull(), and nothing will break with this patch even
> >>> if that is "/dev/null", but I thought I'd double check.
> >>>
> >>> Thanks.
> >>>
> > This problem originally showed up in the
> > https://github.com/so-fancy/diff-so-fancy project, which has a copy of
> > DiffHighlight.pm.   That project allows diffsofancy (perl) to be run
> > from the command line without requiring the bash environment ((well ,
> > sort of) including the associated perl).
> 
> Thanks for additional comments.  
> 
> In any case, Windows is not my bailiwick, so I'll hope that the
> above comments from you would help Dscho in his response and wait.
> I know use of File::Spec->devnull() won't hurt POSIX folks so making
> sure this won't break Git for Windows is the primary thing I woudl
> worry about this patch.

Indeed, the patch in question regards something I consider outside Git for
Windows' realm. As Chris said, you can run this script from a PowerShell
prompt, without any Git Bash (and without Git's Perl) involved.

I am fine with this patch, as long as the author name is fixed to match
the name in the Signed-off-by: footer ;-) [*1*]

Ciao,
Dscho

Footnote *1*: This patch came in via GitGitGadget, and if I had infinite
amounts of time, I would probably implement some rudimentary pre-checks,
such as: does the Author: header match the first Signed-off-by: footer, is
the commit message wrapped correctly, does the oneline have a prefix and
continues lower-case, etc. And GitGitGadget would then point out the
issues, possibly even try to fix them and push up the fixed commits.

If anybody agrees with these goals and is curious enough to dive into some
Typescript programming, I'd be very happy to guide that person through
implementing this ;-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-31  5:07   ` Junio C Hamano
@ 2018-10-31 11:14     ` Johannes Schindelin
  0 siblings, 0 replies; 18+ messages in thread
From: Johannes Schindelin @ 2018-10-31 11:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: chris via GitGitGadget, git, chris

Hi,

On Wed, 31 Oct 2018, Junio C Hamano wrote:

> > From: chris <chris@webstech.net>
> 
> Please make this line read like
> 
> 	From: Chris Webster <chris@webstech.net>
> 
> i.e. the author should be the person who is signing off that patch.

This is most likely recorded as the commit's author in the commit
object... Chris, to fix it, make sure that your `user.name` is configured
correctly, and then call `git commit --amend --reset-author`.

> > Use File::Spec->devnull() for output redirection to avoid messages
> > when Windows version of Perl is first in path.  The message 'The
> > system cannot find the path specified.' is displayed each time git is
> > run to get colors.
> >
> > Signed-off-by: Chris. Webster <chris@webstech.net>
> > ---
> >  contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> There are a handful more instances of /dev/null found if you do
> 
> 	$ git grep /dev/null -- \*.pl \*.pm
> 
> The one in perl/Git.pm must be shared by scripts written in Perl, so
> it may be worth giving the same tweak to it, like this patch does to
> the highlight script.

I do not think that perl/Git.pm is intended to run with any random Perl
interpreter. It has to be one that has been verified to work correctly
with the Perl code in perl/, and that code is notoriously reliant on POSIX
behavior, hence our choice to go with MSYS2 Perl (there *is* a MINGW Perl
package in Git for Windows' SDK, but it will most likely not work, in
particular because of the missing Subversion bindings).

So I would restrict the search to contrib/\*.pl, contrib/\*.perl and
contrib/\*.pm. The stuff in contrib/ is supposed to be semi-independent
from the particular Git one is using (and from whatever Perl is shipped
with it, if any).

Ciao,
Johannes

> > diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
> > index 536754583..7440aa1c4 100644
> > --- a/contrib/diff-highlight/DiffHighlight.pm
> > +++ b/contrib/diff-highlight/DiffHighlight.pm
> > @@ -4,6 +4,11 @@ use 5.008;
> >  use warnings FATAL => 'all';
> >  use strict;
> >  
> > +# Use the correct value for both UNIX and Windows (/dev/null vs nul)
> > +use File::Spec;
> > +
> > +my $NULL = File::Spec->devnull();
> > +
> >  # Highlight by reversing foreground and background. You could do
> >  # other things like bold or underline if you prefer.
> >  my @OLD_HIGHLIGHT = (
> > @@ -134,7 +139,7 @@ sub highlight_stdin {
> >  # fallback, which means we will work even if git can't be run.
> >  sub color_config {
> >  	my ($key, $default) = @_;
> > -	my $s = `git config --get-color $key 2>/dev/null`;
> > +	my $s = `git config --get-color $key 2>$NULL`;
> >  	return length($s) ? $s : $default;
> >  }
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 0/1] DiffHighlight.pm: Use correct /dev/null for UNIX and Windows
  2018-10-30 18:26 [PATCH 0/1] DiffHighlight.pm: Use correct /dev/null for UNIX and Windows Chris. Webster via GitGitGadget
  2018-10-30 18:26 ` [PATCH 1/1] " chris via GitGitGadget
@ 2018-10-31 22:58 ` Chris. Webster via GitGitGadget
       [not found]   ` <bcbffa141116f869db40e4572f9824a3d090c20c.1541026721.git.gitgitgadget@gmail.com>
  1 sibling, 1 reply; 18+ messages in thread
From: Chris. Webster via GitGitGadget @ 2018-10-31 22:58 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Use File::Spec->devnull() for output redirection to avoid messages when
Windows version of Perl is first in path. The message 'The system cannot
find the path specified.' is displayed each time git is run to get colors.

Chris. Webster (1):
  diff-highlight: Use correct /dev/null for UNIX and Windows

 contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


base-commit: c670b1f876521c9f7cd40184bf7ed05aad843433
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-59%2Fwebstech%2Fmaster-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-59/webstech/master-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/59

Range-diff vs v1:

 1:  8159cbd1b8 ! 1:  bcbffa1411 Use correct /dev/null for UNIX and Windows
     @@ -1,6 +1,6 @@
     -Author: chris <chris@webstech.net>
     +Author: Chris. Webster <chris@webstech.net>
      
     -    Use correct /dev/null for UNIX and Windows
     +    diff-highlight: Use correct /dev/null for UNIX and Windows
      
          Use File::Spec->devnull() for output redirection to avoid messages
          when Windows version of Perl is first in path.  The message 'The

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/1] Use correct /dev/null for UNIX and Windows
  2018-10-31 11:10           ` Johannes Schindelin
@ 2018-11-01  1:57             ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2018-11-01  1:57 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Chris Webster, gitgitgadget, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Indeed, the patch in question regards something I consider outside Git for
> Windows' realm. As Chris said, you can run this script from a PowerShell
> prompt, without any Git Bash (and without Git's Perl) involved.
>
> I am fine with this patch, as long as the author name is fixed to match
> the name in the Signed-off-by: footer ;-) [*1*]

Thanks, I'll find a corrected patch on the list (or manufacture it
out of the original) and queue, then.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/1] diff-highlight: Use correct /dev/null for UNIX and Windows
       [not found]   ` <bcbffa141116f869db40e4572f9824a3d090c20c.1541026721.git.gitgitgadget@gmail.com>
@ 2018-11-06 14:01     ` Johannes Schindelin
  2019-05-07  4:18       ` Chris Webster
  2019-05-08 10:45     ` Fwd: " Git Gadget
  1 sibling, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2018-11-06 14:01 UTC (permalink / raw)
  To: Chris. Webster via GitGitGadget; +Cc: git, Junio C Hamano, Chris. Webster

List,

I have no idea why this mail made it to GitGitGadget's email account but
not to the Git mailing list... Sorry about that.

Ciao,
Johannes

On Wed, 31 Oct 2018, Chris. Webster via GitGitGadget wrote:

> From: "Chris. Webster" <chris@webstech.net>
> 
> Use File::Spec->devnull() for output redirection to avoid messages
> when Windows version of Perl is first in path.  The message 'The
> system cannot find the path specified.' is displayed each time git is
> run to get colors.
> 
> Signed-off-by: Chris. Webster <chris@webstech.net>
> ---
>  contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
> index 536754583b..7440aa1c46 100644
> --- a/contrib/diff-highlight/DiffHighlight.pm
> +++ b/contrib/diff-highlight/DiffHighlight.pm
> @@ -4,6 +4,11 @@ use 5.008;
>  use warnings FATAL => 'all';
>  use strict;
>  
> +# Use the correct value for both UNIX and Windows (/dev/null vs nul)
> +use File::Spec;
> +
> +my $NULL = File::Spec->devnull();
> +
>  # Highlight by reversing foreground and background. You could do
>  # other things like bold or underline if you prefer.
>  my @OLD_HIGHLIGHT = (
> @@ -134,7 +139,7 @@ sub highlight_stdin {
>  # fallback, which means we will work even if git can't be run.
>  sub color_config {
>  	my ($key, $default) = @_;
> -	my $s = `git config --get-color $key 2>/dev/null`;
> +	my $s = `git config --get-color $key 2>$NULL`;
>  	return length($s) ? $s : $default;
>  }
>  
> -- 
> gitgitgadget
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/1] diff-highlight: Use correct /dev/null for UNIX and Windows
  2018-11-06 14:01     ` [PATCH v2 1/1] diff-highlight: " Johannes Schindelin
@ 2019-05-07  4:18       ` Chris Webster
  2019-05-12 20:24         ` Philip Oakley
  0 siblings, 1 reply; 18+ messages in thread
From: Chris Webster @ 2019-05-07  4:18 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Chris. Webster via GitGitGadget, git, Junio C Hamano

I know these can take some time but is this pending any update from
me?  The accepted changes will be merged back into the diff-so-fancy
project.

There was a question about other uses of /dev/null.  In the contrib
directory, there are a couple of uses.

contrib/buildsystems/engine.pl - not clear if this is still of use or
always expects to always be running in a mingw type environment.
contrib/mw-to-git/git-remote-mediawiki.perl - this is cloned from a
separately maintained github project.  Should any changes be issues on
that project?

thanks,
...chris.

On Tue, Nov 6, 2018 at 6:02 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> List,
>
> I have no idea why this mail made it to GitGitGadget's email account but
> not to the Git mailing list... Sorry about that.
>
> Ciao,
> Johannes
>
> On Wed, 31 Oct 2018, Chris. Webster via GitGitGadget wrote:
>
> > From: "Chris. Webster" <chris@webstech.net>
> >
> > Use File::Spec->devnull() for output redirection to avoid messages
> > when Windows version of Perl is first in path.  The message 'The
> > system cannot find the path specified.' is displayed each time git is
> > run to get colors.
> >
> > Signed-off-by: Chris. Webster <chris@webstech.net>
> > ---
> >  contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
> > index 536754583b..7440aa1c46 100644
> > --- a/contrib/diff-highlight/DiffHighlight.pm
> > +++ b/contrib/diff-highlight/DiffHighlight.pm
> > @@ -4,6 +4,11 @@ use 5.008;
> >  use warnings FATAL => 'all';
> >  use strict;
> >
> > +# Use the correct value for both UNIX and Windows (/dev/null vs nul)
> > +use File::Spec;
> > +
> > +my $NULL = File::Spec->devnull();
> > +
> >  # Highlight by reversing foreground and background. You could do
> >  # other things like bold or underline if you prefer.
> >  my @OLD_HIGHLIGHT = (
> > @@ -134,7 +139,7 @@ sub highlight_stdin {
> >  # fallback, which means we will work even if git can't be run.
> >  sub color_config {
> >       my ($key, $default) = @_;
> > -     my $s = `git config --get-color $key 2>/dev/null`;
> > +     my $s = `git config --get-color $key 2>$NULL`;
> >       return length($s) ? $s : $default;
> >  }
> >
> > --
> > gitgitgadget
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Fwd: [PATCH v2 1/1] diff-highlight: Use correct /dev/null for UNIX and Windows
       [not found]   ` <bcbffa141116f869db40e4572f9824a3d090c20c.1541026721.git.gitgitgadget@gmail.com>
  2018-11-06 14:01     ` [PATCH v2 1/1] diff-highlight: " Johannes Schindelin
@ 2019-05-08 10:45     ` Git Gadget
  2019-05-09  3:19       ` Junio C Hamano
  1 sibling, 1 reply; 18+ messages in thread
From: Git Gadget @ 2019-05-08 10:45 UTC (permalink / raw)
  To: git, Junio C Hamano, chris

Forwarding this mail to the Git mailing list, as the original did not
make it there (for reasons unknown).

---------- Forwarded message ---------
From: Chris. Webster via GitGitGadget <gitgitgadget@gmail.com>
Date: Wed, Oct 31, 2018 at 11:58 PM
Subject: [PATCH v2 1/1] diff-highlight: Use correct /dev/null for UNIX
and Windows
To: <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>, Chris. Webster <chris@webstech.net>


From: "Chris. Webster" <chris@webstech.net>

Use File::Spec->devnull() for output redirection to avoid messages
when Windows version of Perl is first in path.  The message 'The
system cannot find the path specified.' is displayed each time git is
run to get colors.

Signed-off-by: Chris. Webster <chris@webstech.net>
---
 contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/contrib/diff-highlight/DiffHighlight.pm
b/contrib/diff-highlight/DiffHighlight.pm
index 536754583b..7440aa1c46 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -4,6 +4,11 @@ use 5.008;
 use warnings FATAL => 'all';
 use strict;

+# Use the correct value for both UNIX and Windows (/dev/null vs nul)
+use File::Spec;
+
+my $NULL = File::Spec->devnull();
+
 # Highlight by reversing foreground and background. You could do
 # other things like bold or underline if you prefer.
 my @OLD_HIGHLIGHT = (
@@ -134,7 +139,7 @@ sub highlight_stdin {
 # fallback, which means we will work even if git can't be run.
 sub color_config {
        my ($key, $default) = @_;
-       my $s = `git config --get-color $key 2>/dev/null`;
+       my $s = `git config --get-color $key 2>$NULL`;
        return length($s) ? $s : $default;
 }

--
gitgitgadget

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Fwd: [PATCH v2 1/1] diff-highlight: Use correct /dev/null for UNIX and Windows
  2019-05-08 10:45     ` Fwd: " Git Gadget
@ 2019-05-09  3:19       ` Junio C Hamano
  2019-05-09 18:52         ` Johannes Schindelin
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2019-05-09  3:19 UTC (permalink / raw)
  To: Git Gadget; +Cc: git, chris

Git Gadget <gitgitgadget@gmail.com> writes:

> Forwarding this mail to the Git mailing list, as the original did not
> make it there (for reasons unknown).

It seems that the forwarding mechansim (if this weren't manual---I
cannot tell) mangles whitespaces?  No need to resend, as the
receiving end manually fixed them up.

Thanks.

>
> ---------- Forwarded message ---------
> From: Chris. Webster via GitGitGadget <gitgitgadget@gmail.com>
> Date: Wed, Oct 31, 2018 at 11:58 PM
> Subject: [PATCH v2 1/1] diff-highlight: Use correct /dev/null for UNIX
> and Windows
> To: <git@vger.kernel.org>
> Cc: Junio C Hamano <gitster@pobox.com>, Chris. Webster <chris@webstech.net>
>
>
> From: "Chris. Webster" <chris@webstech.net>
>
> Use File::Spec->devnull() for output redirection to avoid messages
> when Windows version of Perl is first in path.  The message 'The
> system cannot find the path specified.' is displayed each time git is
> run to get colors.
>
> Signed-off-by: Chris. Webster <chris@webstech.net>
> ---
>  contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/diff-highlight/DiffHighlight.pm
> b/contrib/diff-highlight/DiffHighlight.pm
> index 536754583b..7440aa1c46 100644
> --- a/contrib/diff-highlight/DiffHighlight.pm
> +++ b/contrib/diff-highlight/DiffHighlight.pm
> @@ -4,6 +4,11 @@ use 5.008;
>  use warnings FATAL => 'all';
>  use strict;
>
> +# Use the correct value for both UNIX and Windows (/dev/null vs nul)
> +use File::Spec;
> +
> +my $NULL = File::Spec->devnull();
> +
>  # Highlight by reversing foreground and background. You could do
>  # other things like bold or underline if you prefer.
>  my @OLD_HIGHLIGHT = (
> @@ -134,7 +139,7 @@ sub highlight_stdin {
>  # fallback, which means we will work even if git can't be run.
>  sub color_config {
>         my ($key, $default) = @_;
> -       my $s = `git config --get-color $key 2>/dev/null`;
> +       my $s = `git config --get-color $key 2>$NULL`;
>         return length($s) ? $s : $default;
>  }
>
> --
> gitgitgadget

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Fwd: [PATCH v2 1/1] diff-highlight: Use correct /dev/null for UNIX and Windows
  2019-05-09  3:19       ` Junio C Hamano
@ 2019-05-09 18:52         ` Johannes Schindelin
  0 siblings, 0 replies; 18+ messages in thread
From: Johannes Schindelin @ 2019-05-09 18:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Gadget, git, chris

Hi Junio,

On Thu, 9 May 2019, Junio C Hamano wrote:

> Git Gadget <gitgitgadget@gmail.com> writes:
>
> > Forwarding this mail to the Git mailing list, as the original did not
> > make it there (for reasons unknown).
>
> It seems that the forwarding mechansim (if this weren't manual---I
> cannot tell) mangles whitespaces?  No need to resend, as the
> receiving end manually fixed them up.

I did send it manually, via the GMail interface (which I seem to be unable
to use effectively).

Sorry for the trouble, and thanks for fixing it up.

For the record, you could also always just fetch from the tag. The mails
GitGitGadget sends (unless I manually forward mails) are generated from
those tags, so (modulo bugs) the sent patch and the commit(s) reachable
from the published tag are identical.

In this instance, it would have been:

	git fetch https://github.com/gitgitgadget/git \
		pr-59/webstech/master-v2

Thank you,
Dscho

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/1] diff-highlight: Use correct /dev/null for UNIX and Windows
  2019-05-07  4:18       ` Chris Webster
@ 2019-05-12 20:24         ` Philip Oakley
  0 siblings, 0 replies; 18+ messages in thread
From: Philip Oakley @ 2019-05-12 20:24 UTC (permalink / raw)
  To: Chris Webster, Johannes Schindelin
  Cc: Chris. Webster via GitGitGadget, git, Junio C Hamano

Hi Chris,


On 07/05/2019 05:18, Chris Webster wrote:
> I know these can take some time but is this pending any update from
> me?  The accepted changes will be merged back into the diff-so-fancy
> project.
>
> There was a question about other uses of /dev/null.  In the contrib
> directory, there are a couple of uses.
>
> contrib/buildsystems/engine.pl - not clear if this is still of use or
> always expects to always be running in a mingw type environment.
While the `contrib/buildsystems/engine.pl ` is not often used, it does 
provide a route for users of older Visual studio version.
I'm not sure if the mingw environment is relevant to its operation - I'd 
expect it (the generation of an .sln that would compile) to still work 
with the current MSYS2 basis, but the `install` process was never fully 
completed (an probably isn't a worthwhile project unless you have the 
particular itch to scratch).

The newer MSVC=1 build process using vcpkg has solved most of the 
issues, but I'm getting a couple of issues with some of the 
libraries/include paths with the latest VS2017, see issue #2186 / #2179 
/ https://groups.google.com/forum/#!topic/git-for-windows/Y99a0dzlVJY , 
though hopefully I'll get them sorted this week.

> contrib/mw-to-git/git-remote-mediawiki.perl - this is cloned from a
> separately maintained github project.  Should any changes be issues on
> that project?
>
> thanks,
> ...chris.
>
> On Tue, Nov 6, 2018 at 6:02 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> List,
>>
>> I have no idea why this mail made it to GitGitGadget's email account but
>> not to the Git mailing list... Sorry about that.
>>
>> Ciao,
>> Johannes
>>
>> On Wed, 31 Oct 2018, Chris. Webster via GitGitGadget wrote:
>>
>>> From: "Chris. Webster" <chris@webstech.net>
>>>
>>> Use File::Spec->devnull() for output redirection to avoid messages
>>> when Windows version of Perl is first in path.  The message 'The
>>> system cannot find the path specified.' is displayed each time git is
>>> run to get colors.
>>>
>>> Signed-off-by: Chris. Webster <chris@webstech.net>
>>> ---
>>>   contrib/diff-highlight/DiffHighlight.pm | 7 ++++++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
>>> index 536754583b..7440aa1c46 100644
>>> --- a/contrib/diff-highlight/DiffHighlight.pm
>>> +++ b/contrib/diff-highlight/DiffHighlight.pm
>>> @@ -4,6 +4,11 @@ use 5.008;
>>>   use warnings FATAL => 'all';
>>>   use strict;
>>>
>>> +# Use the correct value for both UNIX and Windows (/dev/null vs nul)
>>> +use File::Spec;
>>> +
>>> +my $NULL = File::Spec->devnull();
>>> +
>>>   # Highlight by reversing foreground and background. You could do
>>>   # other things like bold or underline if you prefer.
>>>   my @OLD_HIGHLIGHT = (
>>> @@ -134,7 +139,7 @@ sub highlight_stdin {
>>>   # fallback, which means we will work even if git can't be run.
>>>   sub color_config {
>>>        my ($key, $default) = @_;
>>> -     my $s = `git config --get-color $key 2>/dev/null`;
>>> +     my $s = `git config --get-color $key 2>$NULL`;
>>>        return length($s) ? $s : $default;
>>>   }
>>>
>>> --
>>> gitgitgadget
>>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-05-12 20:24 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-30 18:26 [PATCH 0/1] DiffHighlight.pm: Use correct /dev/null for UNIX and Windows Chris. Webster via GitGitGadget
2018-10-30 18:26 ` [PATCH 1/1] " chris via GitGitGadget
2018-10-31  3:56   ` Jeff King
2018-10-31  4:48     ` Junio C Hamano
2018-10-31  4:54   ` Junio C Hamano
     [not found]     ` <CAGT1KpWoGD0xgTrC-+X1WqY_M=2arYbs4ZX6Nnj-zHK6mgu+nw@mail.gmail.com>
2018-10-31  5:41       ` Chris Webster
2018-10-31  6:08         ` Junio C Hamano
2018-10-31 11:10           ` Johannes Schindelin
2018-11-01  1:57             ` Junio C Hamano
2018-10-31  5:07   ` Junio C Hamano
2018-10-31 11:14     ` Johannes Schindelin
2018-10-31 22:58 ` [PATCH v2 0/1] DiffHighlight.pm: " Chris. Webster via GitGitGadget
     [not found]   ` <bcbffa141116f869db40e4572f9824a3d090c20c.1541026721.git.gitgitgadget@gmail.com>
2018-11-06 14:01     ` [PATCH v2 1/1] diff-highlight: " Johannes Schindelin
2019-05-07  4:18       ` Chris Webster
2019-05-12 20:24         ` Philip Oakley
2019-05-08 10:45     ` Fwd: " Git Gadget
2019-05-09  3:19       ` Junio C Hamano
2019-05-09 18:52         ` Johannes Schindelin

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).