RFR: CODETOOLS-7902478 - Add webrev flag to avoid printing workspace (Was: Ability to not display local filesystem path in the generated webrev?)

[Jonathan Gibbons]

Jesper,

I'll look at sponsoring this change into the repo tomorrow, Thursday.

-- Jon

On 6/12/19 8:57 PM, Jaikiran Pai wrote:
> I'm not a reviewer, but this change looks good to me. I applied this
> patch locally and ran the tool with both -w and without it. Works as
> expected.
>
> Thank you Jesper for implementing this.
>
> -Jaikiran
>
> On 13/06/19 3:48 AM, jesper.wilhelmsson at oracle.com wrote:
>>> On 12 Jun 2019, at 15:02, Jaikiran Pai <jai.forums2013 at gmail.com> wrote:
>>>
>>> Hello,
>>>
>>> First time user of the webrev tool - I happened to use it today for one
>>> of the patches I propose for a bug fix. In the generated webrev html
>>> index file, I see that there's a "Workspace:" whose value points to the
>>> local filesystem path of my system. I looked at the options available
>>> for the webrev.ksh script (and even the code in the script) but couldn't
>>> find a way to disable generating this (relatively private) information
>>> in the webrev.
>>>
>>> Is there a reason to have this information exposed in the webrev? If
>>> not, can a option be added to disable generating this?
>>>
>>> -Jaikiran
>> It seems to me that this is a relic from a time where all users of webrev was located on the same filesystem. Today I would expect most users to run webrev on a local filesystem and the path to the workspace (and also the path to webrev at the bottom of the page) are of less value. I filed CODETOOLS-7902478 to add a flag to disable this output. The patch below fixes it. Please review.
>>
>> Thanks
>> /Jesper
>>
>>
>> diff --git a/webrev.ksh b/webrev.ksh
>> old mode 100644
>> new mode 100755
>> --- a/webrev.ksh
>> +++ b/webrev.ksh
>> @@ -1812,6 +1812,7 @@
>>           -u <username>: Use that username instead of 'guessing' one.
>>   	-m: Forces the use of Mercurial
>>   	-C: Don't show comments
>> +	-w: Don't display local paths to workspace or webrev installation
>>
>>   Mercurial only options:
>>   	-r rev: Compare against a specified revision
>> @@ -1904,7 +1905,8 @@
>>   Nflag=
>>   Cflag=
>>   forestflag=
>> -while getopts "c:i:o:p:r:u:mONvfbC" opt
>> +print_paths=1
>> +while getopts "c:i:o:p:r:u:mONvfbCw" opt
>>   do
>>   	case $opt in
>>           b)      bflag=1;;
>> @@ -1942,6 +1944,7 @@
>>
>>   	v)	print "$0 version: $WEBREV_UPDATED";;
>>
>> +	w)	print_paths="";;
>>
>>   	?)	usage;;
>>   	esac
>> @@ -2706,7 +2709,9 @@
>>   	print "<tr><th>Prepared by:</th><td>$username $date</td></tr>"
>>   fi
>>
>> +if [[ -n "$print_paths" ]]; then
>>   print "<tr><th>Workspace:</th><td>$CWS</td></tr>"
>> +fi
>>   if [[ -n $parent_webrev ]]; then
>>           print "<tr><th>Compare against:</th><td>"
>>   	print "webrev at $parent_webrev"
>> @@ -2916,7 +2921,12 @@
>>   print
>>   print "<hr />"
>>   print "<p style=\"font-size: small\">"
>> -print "This code review page was prepared using <b>$0</b>"
>> +print "This code review page was prepared using "
>> +if [[ -n "$print_paths" ]]; then
>> +  print "<b>$0</b>"
>> +else
>> +  print "webrev"
>> +fi
>>   print "(vers $WEBREV_UPDATED)."
>>   print "</body>"
>>   print "</html>"
>>
>>