[PATCH] doc: Don't echo sed command for manpage-base-url.xsl

[PATCH] doc: Don't echo sed command for manpage-base-url.xsl

[Tim Schumacher]

Previously, the sed command for generating manpage-base-url.xsl
was printed to the console when being run.

For the purpose of silencing it, define a $(QUIET) variable which
contains an '@' if verbose mode isn't enabled and which is empty
otherwise. This just silences the command invocation without doing
anything else.

Signed-off-by: Tim Schumacher <timschumi@gmx.de>
---
 Documentation/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index a42dcfc74..45454e9b5 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -217,6 +217,7 @@ endif
 
 ifneq ($(findstring $(MAKEFLAGS),s),s)
 ifndef V
+	QUIET		= @
 	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
 	QUIET_XMLTO	= @echo '   ' XMLTO $@;
 	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
@@ -344,7 +345,7 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf
 	mv $@+ $@
 
 manpage-base-url.xsl: manpage-base-url.xsl.in
-	sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
+	$(QUIET)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
 
 %.1 %.5 %.7 : %.xml manpage-base-url.xsl
 	$(QUIET_XMLTO)$(RM) $@ && \
-- 
2.19.0.rc0

Re: [PATCH] doc: Don't echo sed command for manpage-base-url.xsl

[Eric Sunshine]

On Tue, Aug 28, 2018 at 5:21 PM Tim Schumacher <timschumi@gmx.de> wrote:
> Previously, the sed command for generating manpage-base-url.xsl
> was printed to the console when being run.
>
> For the purpose of silencing it, define a $(QUIET) variable which
> contains an '@' if verbose mode isn't enabled and which is empty
> otherwise. This just silences the command invocation without doing
> anything else.

One thing that is missing from this commit message is the explanation
of _why_ this is desirable. And, why only this command? Without
understanding the underlying reason(s), it's hard to judge the value
of this change.

Thanks.

Re: [PATCH] doc: Don't echo sed command for manpage-base-url.xsl

[Junio C Hamano]

Tim Schumacher <timschumi@gmx.de> writes:

> Previously, the sed command for generating manpage-base-url.xsl
> was printed to the console when being run.
>
> For the purpose of silencing it, define a $(QUIET) variable which
> contains an '@' if verbose mode isn't enabled and which is empty
> otherwise. This just silences the command invocation without doing
> anything else.
>
> Signed-off-by: Tim Schumacher <timschumi@gmx.de>
> ---

I am not sure if this is a good change.  All these QUIET_$TOOL hide
details of running the $TOOL to produce the final output of the step,
but they still do report what they are creating via which $TOOL.

Shouldn't the step to create manpage-base-url.xsl be the same?  The
detail of creating it (i.e. token @@MAN_BASE_URL@@ is replaced with
the actual value) may want to be squelched, but shouldn't we still
be reporting that we are creating manpage-base-url.xsl file?

>  Documentation/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index a42dcfc74..45454e9b5 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -217,6 +217,7 @@ endif
>  
>  ifneq ($(findstring $(MAKEFLAGS),s),s)
>  ifndef V
> +	QUIET		= @
>  	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
>  	QUIET_XMLTO	= @echo '   ' XMLTO $@;
>  	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
> @@ -344,7 +345,7 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf
>  	mv $@+ $@
>  
>  manpage-base-url.xsl: manpage-base-url.xsl.in
> -	sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
> +	$(QUIET)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
>  
>  %.1 %.5 %.7 : %.xml manpage-base-url.xsl
>  	$(QUIET_XMLTO)$(RM) $@ && \

[PATCH v2] doc: Don't echo sed command for manpage-base-url.xsl

[Tim Schumacher]

Previously, the sed command for generating manpage-base-url.xsl
was printed to the console when being run.

Make the console output for this rule similiar to all the
other rules by printing a short status message instead of
the whole command.

Signed-off-by: Tim Schumacher <timschumi@gmx.de>
---

To Junio C Hamano:
The rule does now print "SED manpage-base-url.xsl"
to the console, which is similiar to other QUIET_$TOOL
rules.

To Eric Sunshine:
I reworded the commit message to focus more on _why_
the patch is relevant.

---

 Documentation/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index a42dcfc74..cbf33c5a7 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -225,6 +225,7 @@ ifndef V
 	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
 	QUIET_GEN	= @echo '   ' GEN $@;
 	QUIET_LINT	= @echo '   ' LINT $@;
+	QUIET_SED	= @echo '   ' SED $@;
 	QUIET_STDERR	= 2> /dev/null
 	QUIET_SUBDIR0	= +@subdir=
 	QUIET_SUBDIR1	= ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
@@ -344,7 +345,7 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf
 	mv $@+ $@
 
 manpage-base-url.xsl: manpage-base-url.xsl.in
-	sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
+	$(QUIET_SED)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
 
 %.1 %.5 %.7 : %.xml manpage-base-url.xsl
 	$(QUIET_XMLTO)$(RM) $@ && \
-- 
2.19.0.rc1.1.g093671f86

[PATCH v3] doc: Don't echo sed command for manpage-base-url.xsl

[Tim Schumacher]

Previously, the sed command for generating manpage-base-url.xsl
was printed to the console when being run.

Make the console output for this rule similiar to all the
other rules by printing a short status message instead of
the whole command.

Signed-off-by: Tim Schumacher <timschumi@gmx.de>
---
 Documentation/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index a42dcfc74..95f6a321f 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -344,7 +344,7 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf
 	mv $@+ $@
 
 manpage-base-url.xsl: manpage-base-url.xsl.in
-	sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
+	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
 
 %.1 %.5 %.7 : %.xml manpage-base-url.xsl
 	$(QUIET_XMLTO)$(RM) $@ && \
-- 
2.19.0.rc1.1.g093671f86

Re: [PATCH v2] doc: Don't echo sed command for manpage-base-url.xsl

[Jonathan Nieder]

Tim Schumacher wrote:

> Subject: doc: Don't echo sed command for manpage-base-url.xsl

At first glance, I thought this was going to change the rendered
documentation in some way.  Maybe this should say

	Makefile: make 'sed' commands quieter

?  That led me to look in the Makefile, where it appears we already do
this for some sed commands, using QUIET_GEN.  What would you think of
using the same for manpage-base-url.xsl?

Thanks,
Jonathan

Re: [PATCH v3] doc: Don't echo sed command for manpage-base-url.xsl

[Jonathan Nieder]

Tim Schumacher wrote:

> Subject: doc: Don't echo sed command for manpage-base-url.xsl

Cribbing from my review of v2: a description like

	Documentation/Makefile: make manpage-base-url.xsl generation quieter

would make it more obvious what this does when viewed in "git log
--oneline".

> Previously, the sed command for generating manpage-base-url.xsl
> was printed to the console when being run.
>
> Make the console output for this rule similiar to all the
> other rules by printing a short status message instead of
> the whole command.
>
> Signed-off-by: Tim Schumacher <timschumi@gmx.de>
> ---
>  Documentation/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Oh!  Ignore my reply to v2; looks like you anticipated what I was
going to suggest already.  For next time, if you include a note about
what changed between versions after the --- delimiter, that can help
save some time.

With or without the suggested commit message tweak,

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thank you.

Re: [PATCH v3] doc: Don't echo sed command for manpage-base-url.xsl

[Tim Schumacher]

On 29.08.18 18:55, Jonathan Nieder wrote:
> Tim Schumacher wrote:
> 
>> Subject: doc: Don't echo sed command for manpage-base-url.xsl
> 
> Cribbing from my review of v2: a description like
> 
> 	Documentation/Makefile: make manpage-base-url.xsl generation quieter
> 
> would make it more obvious what this does when viewed in "git log
> --oneline".

imho, the "Documentation/Makefile" is a bit too long (about 1/3 of the
first line). Would it be advisable to keep the previous prefix of "doc"
(which would shorten down the line from 68 characters down to 49) and
to use only the second part of your proposed first line? Depending on
the response I would address this in v4 of this patch.

> 
>> Previously, the sed command for generating manpage-base-url.xsl
>> was printed to the console when being run.
>>
>> Make the console output for this rule similiar to all the
>> other rules by printing a short status message instead of
>> the whole command.
>>
>> Signed-off-by: Tim Schumacher <timschumi@gmx.de>
>> ---
>>   Documentation/Makefile | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Oh!  Ignore my reply to v2; looks like you anticipated what I was
> going to suggest already.  For next time, if you include a note about
> what changed between versions after the --- delimiter, that can help
> save some time.
> 

The change to QUIET_GEN was proposed by Junio, but that E-Mail
wasn't CC'ed to the mailing list, probably due to him typing
the response on a phone.

I originally included a note about the change as well, but I
forgot to copy it over to the new patch after I generated a
second version of v3.

> With or without the suggested commit message tweak,
> 
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> 
> Thank you.
> 

Thanks for the review!

Re: [PATCH v3] doc: Don't echo sed command for manpage-base-url.xsl

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Tim Schumacher wrote:
>
>> Subject: doc: Don't echo sed command for manpage-base-url.xsl
>
> Cribbing from my review of v2: a description like
>
> 	Documentation/Makefile: make manpage-base-url.xsl generation quieter
>
> would make it more obvious what this does when viewed in "git log
> --oneline".

Sounds good; let's take it.

>> Previously, the sed command for generating manpage-base-url.xsl
>> was printed to the console when being run.

The convention is that we talk about the state before the current
series in question is applied in the present tense, so "previously"
is not needed.  Perhaps

    The exact sed command to generate manpage-base-url.xsl appears in
    the output, unlike the rules for other files that by default only
    show summary.

is sufficient.  The output is not always going to "the console", and
it is not like we change behaviour depending on where the output is
going, so it is misleading to say "the console" (iow, the phrase "to
the console" has negative information density in the above
sentence).

>> Make the console output for this rule similiar to all the
>> other rules by printing a short status message instead of
>> the whole command.

Likewise, s/console //;

I'll all do the above tweaks while queueing.

Thanks, both.

>>
>> Signed-off-by: Tim Schumacher <timschumi@gmx.de>
>> ---
>>  Documentation/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Oh!  Ignore my reply to v2; looks like you anticipated what I was
> going to suggest already.  For next time, if you include a note about
> what changed between versions after the --- delimiter, that can help
> save some time.
>
> With or without the suggested commit message tweak,
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Thank you.