[PATCH] regtool: Ignore /proc/registry{,32,64}/ prefix, with forward or backslashes, allowing path completion

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] regtool: Ignore /proc/registry{,32,64}/ prefix, with forward or backslashes, allowing path completion

Brian Inglis
---
 winsup/utils/regtool.cc | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/winsup/utils/regtool.cc b/winsup/utils/regtool.cc
index a44d90768..ddb1304cd 100644
--- a/winsup/utils/regtool.cc
+++ b/winsup/utils/regtool.cc
@@ -167,7 +167,9 @@ usage (FILE *where = stderr)
       "  users    HKU   HKEY_USERS\n"
       "\n"
       "If the keyname starts with a forward slash ('/'), the forward slash is used\n"
-      "as separator and the backslash can be used as escape character.\n");
+      "as separator and the backslash can be used as escape character.\n"
+      "If the keyname starts with /proc/registry{,32,64}/, using forward or backward\n"
+      "slashes, allowing path completion, that part of the prefix is ignored.\n");
       fprintf (where, ""
       "Example:\n"
       "%s list '/machine/SOFTWARE/Classes/MIME/Database/Content Type/audio\\/wav'\n\n", prog_name);
@@ -350,6 +352,15 @@ find_key (int howmanyparts, REGSAM access, int option = 0)
       *h = 0;
       n = e;
     }
+  else if (strncmp ("\\proc\\registry", n, strlen ("\\proc\\registry")) == 0)
+    {
+      /* skip /proc/registry{,32,64}/ prefix */
+      n += strlen ("\\proc\\registry");
+      if (strncmp ("64", n, strlen ("64")) == 0)
+        n += strlen ("64");
+      else if (strncmp ("32", n, strlen ("32")) == 0)
+        n += strlen ("32");
+    }
   while (*n != '\\')
     n++;
   *n++ = 0;
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: Ignore /proc/registry{,32,64}/ prefix, with forward or backslashes, allowing path completion

Corinna Vinschen-2
Hi Brian,


The patch idea is nice.  Two nits, though.

Please shorten the commit msg summary line and add a bit of descriptive
text instead.


On Nov 10 09:14, Brian Inglis wrote:

> ---
>  winsup/utils/regtool.cc | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/winsup/utils/regtool.cc b/winsup/utils/regtool.cc
> index a44d90768..ddb1304cd 100644
> --- a/winsup/utils/regtool.cc
> +++ b/winsup/utils/regtool.cc
> @@ -167,7 +167,9 @@ usage (FILE *where = stderr)
>        "  users    HKU   HKEY_USERS\n"
>        "\n"
>        "If the keyname starts with a forward slash ('/'), the forward slash is used\n"
> -      "as separator and the backslash can be used as escape character.\n");
> +      "as separator and the backslash can be used as escape character.\n"
> +      "If the keyname starts with /proc/registry{,32,64}/, using forward or backward\n"
> +      "slashes, allowing path completion, that part of the prefix is ignored.\n");
Is that really essential user information?

I assume this behaviour is something you just expected to work but then
didn't.  With your patch it now works as you expected.  So it's kind of
a bugfix, rather than a change of behaviour the user needs to learn about.

The above text is, IMHO, more confusing than helpful to a user just
asking for regtool --help.  I'd just drop it.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: Ignore /proc/registry{,32,64}/ prefix, with forward or backslashes, allowing path completion

Corinna Vinschen-2
On Nov 11 10:13, Corinna Vinschen wrote:

> Hi Brian,
>
>
> The patch idea is nice.  Two nits, though.
>
> Please shorten the commit msg summary line and add a bit of descriptive
> text instead.
>
>
> On Nov 10 09:14, Brian Inglis wrote:
> > ---
> >  winsup/utils/regtool.cc | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/winsup/utils/regtool.cc b/winsup/utils/regtool.cc
> > index a44d90768..ddb1304cd 100644
> > --- a/winsup/utils/regtool.cc
> > +++ b/winsup/utils/regtool.cc
> > @@ -167,7 +167,9 @@ usage (FILE *where = stderr)
> >        "  users    HKU   HKEY_USERS\n"
> >        "\n"
> >        "If the keyname starts with a forward slash ('/'), the forward slash is used\n"
> > -      "as separator and the backslash can be used as escape character.\n");
> > +      "as separator and the backslash can be used as escape character.\n"
> > +      "If the keyname starts with /proc/registry{,32,64}/, using forward or backward\n"
> > +      "slashes, allowing path completion, that part of the prefix is ignored.\n");
>
> Is that really essential user information?
>
> I assume this behaviour is something you just expected to work but then
> didn't.  With your patch it now works as you expected.  So it's kind of
> a bugfix, rather than a change of behaviour the user needs to learn about.
>
> The above text is, IMHO, more confusing than helpful to a user just
> asking for regtool --help.  I'd just drop it.
In fact, a descriptive sentence like the above would better serve as
part of the commit message, methinks :)


Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: Ignore /proc/registry{,32,64}/ prefix, with forward or backslashes, allowing path completion

Brian Inglis
On 2019-11-11 02:19, Corinna Vinschen wrote:
> On Nov 11 10:13, Corinna Vinschen wrote:
>> On Nov 10 09:14, Brian Inglis wrote:
>> The patch idea is nice.  Two nits, though.
>> Please shorten the commit msg summary line and add a bit of descriptive
>> text instead.

Sorry, I forget and don't notice longer than standard messages, from using
120x60 or larger windows.

>>> ---
>>>  winsup/utils/regtool.cc | 13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/winsup/utils/regtool.cc b/winsup/utils/regtool.cc
>>> index a44d90768..ddb1304cd 100644
>>> --- a/winsup/utils/regtool.cc
>>> +++ b/winsup/utils/regtool.cc
>>> @@ -167,7 +167,9 @@ usage (FILE *where = stderr)
>>>        "  users    HKU   HKEY_USERS\n"
>>>        "\n"
>>>        "If the keyname starts with a forward slash ('/'), the forward slash is used\n"
>>> -      "as separator and the backslash can be used as escape character.\n");
>>> +      "as separator and the backslash can be used as escape character.\n"
>>> +      "If the keyname starts with /proc/registry{,32,64}/, using forward or backward\n"
>>> +      "slashes, allowing path completion, that part of the prefix is ignored.\n");
>>
>> Is that really essential user information?

Absolutely essential!

>> I assume this behaviour is something you just expected to work but then
>> didn't.  With your patch it now works as you expected.  So it's kind of
>> a bugfix, rather than a change of behaviour the user needs to learn about.

To those with similar background or experience it may appear that it should be
supported, but hasn't been until now.

It is definitely not expected behaviour, given how regedit, reg, etc. expect
only hive paths, and how the the current regtool --help reads, clearly expecting
Windows style backslash separated registry paths, probably pasted within single
quotes. That expectation is changed somewhat by the forward slash sentence.
Further changes to expectation needs more documentation.

>> The above text is, IMHO, more confusing than helpful to a user just
>> asking for regtool --help.  I'd just drop it.

It needs documented because it can not in any way be inferred from the existing
regtool ---help, and would not be expected, that it should work. It was never
previously supported or seen as helpful or necessary, so it should be seen as a
non-obvious "surprising" addition, in the opposite sense to "least surprise".

Please someone suggest better wording for the help, as that is the only
documentation available, and is needed, to update existing and inform new users.
Like the code, I tried to maintain the style of the existing help.

As an alternative, how about:
"To support path completion, a keyname prefix of /proc/registry{,32,64}/ is
ignored."

> In fact, a descriptive sentence like the above would better serve as
> part of the commit message, methinks :)

I can definitely --amend that as suggested, but still want to ensure some user
documentation of a non-obvious useful feature. New users, and existing users
that don't subscribe to and read all the announcements and news, will never see
anything different.

Are the cygwin-doc html and man pages generated from the regtool.cc help, or are
there any other sources which need updated?
Now rebuilding current cygwin-doc to be able to check: configures and docs take
quite a long while on a desktop.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: Ignore /proc/registry{,32,64}/ prefix, with forward or backslashes, allowing path completion

Corinna Vinschen-2
On Nov 11 08:30, Brian Inglis wrote:

> On 2019-11-11 02:19, Corinna Vinschen wrote:
> > On Nov 11 10:13, Corinna Vinschen wrote:
> >> On Nov 10 09:14, Brian Inglis wrote:
> >> The patch idea is nice.  Two nits, though.
> >> Please shorten the commit msg summary line and add a bit of descriptive
> >> text instead.
>
> Sorry, I forget and don't notice longer than standard messages, from using
> 120x60 or larger windows.
>
> >>> ---
> >>>  winsup/utils/regtool.cc | 13 ++++++++++++-
> >>>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/winsup/utils/regtool.cc b/winsup/utils/regtool.cc
> >>> index a44d90768..ddb1304cd 100644
> >>> --- a/winsup/utils/regtool.cc
> >>> +++ b/winsup/utils/regtool.cc
> >>> @@ -167,7 +167,9 @@ usage (FILE *where = stderr)
> >>>        "  users    HKU   HKEY_USERS\n"
> >>>        "\n"
> >>>        "If the keyname starts with a forward slash ('/'), the forward slash is used\n"
> >>> -      "as separator and the backslash can be used as escape character.\n");
> >>> +      "as separator and the backslash can be used as escape character.\n"
> >>> +      "If the keyname starts with /proc/registry{,32,64}/, using forward or backward\n"
> >>> +      "slashes, allowing path completion, that part of the prefix is ignored.\n");
> >>
> >> Is that really essential user information?
>
> Absolutely essential!
>
> >> I assume this behaviour is something you just expected to work but then
> >> didn't.  With your patch it now works as you expected.  So it's kind of
> >> a bugfix, rather than a change of behaviour the user needs to learn about.
>
> To those with similar background or experience it may appear that it should be
> supported, but hasn't been until now.
>
> It is definitely not expected behaviour, given how regedit, reg, etc. expect
> only hive paths, and how the the current regtool --help reads, clearly expecting
> Windows style backslash separated registry paths, probably pasted within single
> quotes. That expectation is changed somewhat by the forward slash sentence.
> Further changes to expectation needs more documentation.
>
> >> The above text is, IMHO, more confusing than helpful to a user just
> >> asking for regtool --help.  I'd just drop it.
>
> It needs documented because it can not in any way be inferred from the existing
> regtool ---help, and would not be expected, that it should work. It was never
> previously supported or seen as helpful or necessary, so it should be seen as a
> non-obvious "surprising" addition, in the opposite sense to "least surprise".
>
> Please someone suggest better wording for the help, as that is the only
> documentation available, and is needed, to update existing and inform new users.
> Like the code, I tried to maintain the style of the existing help.
>
> As an alternative, how about:
> "To support path completion, a keyname prefix of /proc/registry{,32,64}/ is
> ignored."
Ok, we can add something to the help text, but the text still sounds
confusing, even the altenative one.  I think the reason is the negative
expression "ignore" here.  Why not express this in a positive way like
this:

  "Use the /proc/registry{,32,64}/ registry path prefix to utilize path
   completion."

Something like that anyway.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH] regtool: allow /proc/registry{,32,64}/ registry path prefix

Brian Inglis
In reply to this post by Brian Inglis
The user can supply the registry path prefix /proc/registry{,32,64}/ to
use path completion.
---
 winsup/doc/utils.xml    |  7 +++++--
 winsup/utils/regtool.cc | 17 ++++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/winsup/doc/utils.xml b/winsup/doc/utils.xml
index 043ed7358..5f266bcb1 100644
--- a/winsup/doc/utils.xml
+++ b/winsup/doc/utils.xml
@@ -1976,8 +1976,11 @@ remote host in either \\hostname or hostname: format and prefix is any of:
   users    HKU   HKEY_USERS
 
 You can use forward slash ('/') as a separator instead of backslash, in
-that case backslash is treated as escape character
-Example: regtool get '\user\software\Microsoft\Clock\iFormat'
+that case backslash is treated as an escape character.
+You can also supply the registry path prefix /proc/registry{,32,64}/ to
+use path completion.
+Example:
+  regtool list '/HKLM/SOFTWARE/Classes/MIME/Database/Content Type/audio\\/wav'
 </screen>
     </refsect1>
 
diff --git a/winsup/utils/regtool.cc b/winsup/utils/regtool.cc
index a44d90768..f91e61d00 100644
--- a/winsup/utils/regtool.cc
+++ b/winsup/utils/regtool.cc
@@ -166,11 +166,13 @@ usage (FILE *where = stderr)
       "  machine  HKLM  HKEY_LOCAL_MACHINE\n"
       "  users    HKU   HKEY_USERS\n"
       "\n"
-      "If the keyname starts with a forward slash ('/'), the forward slash is used\n"
-      "as separator and the backslash can be used as escape character.\n");
+      "You can use forward slash ('/') as a separator instead of backslash, in\n"
+      "that case backslash is treated as an escape character.\n"
+      "You can also supply the registry path prefix /proc/registry{,32,64}/ to\n"
+      "use path completion.\n");
       fprintf (where, ""
       "Example:\n"
-      "%s list '/machine/SOFTWARE/Classes/MIME/Database/Content Type/audio\\/wav'\n\n", prog_name);
+      "%s list '/HKLM/SOFTWARE/Classes/MIME/Database/Content Type/audio\\/wav'\n\n", prog_name);
     }
   if (where == stderr)
     fprintf (where,
@@ -350,6 +352,15 @@ find_key (int howmanyparts, REGSAM access, int option = 0)
       *h = 0;
       n = e;
     }
+  else if (strncmp ("\\proc\\registry", n, strlen ("\\proc\\registry")) == 0)
+    {
+      /* skip /proc/registry{,32,64}/ prefix */
+      n += strlen ("\\proc\\registry");
+      if (strncmp ("64", n, strlen ("64")) == 0)
+        n += strlen ("64");
+      else if (strncmp ("32", n, strlen ("32")) == 0)
+        n += strlen ("32");
+    }
   while (*n != '\\')
     n++;
   *n++ = 0;
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: Ignore /proc/registry{,32,64}/ prefix, with forward or backslashes, allowing path completion

Brian Inglis
In reply to this post by Corinna Vinschen-2
On 2019-11-11 09:28, Corinna Vinschen wrote:

> On Nov 11 08:30, Brian Inglis wrote:
>> On 2019-11-11 02:19, Corinna Vinschen wrote:
>>> On Nov 11 10:13, Corinna Vinschen wrote:
>>>> On Nov 10 09:14, Brian Inglis wrote:
>>>> The patch idea is nice.  Two nits, though.
>>>> Please shorten the commit msg summary line and add a bit of descriptive
>>>> text instead.
>>
>> Sorry, I forget and don't notice longer than standard messages, from using
>> 120x60 or larger windows.
>>
>>>>> ---
>>>>>  winsup/utils/regtool.cc | 13 ++++++++++++-
>>>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/winsup/utils/regtool.cc b/winsup/utils/regtool.cc
>>>>> index a44d90768..ddb1304cd 100644
>>>>> --- a/winsup/utils/regtool.cc
>>>>> +++ b/winsup/utils/regtool.cc
>>>>> @@ -167,7 +167,9 @@ usage (FILE *where = stderr)
>>>>>        "  users    HKU   HKEY_USERS\n"
>>>>>        "\n"
>>>>>        "If the keyname starts with a forward slash ('/'), the forward slash is used\n"
>>>>> -      "as separator and the backslash can be used as escape character.\n");
>>>>> +      "as separator and the backslash can be used as escape character.\n"
>>>>> +      "If the keyname starts with /proc/registry{,32,64}/, using forward or backward\n"
>>>>> +      "slashes, allowing path completion, that part of the prefix is ignored.\n");
>>>>
>>>> Is that really essential user information?
>>
>> Absolutely essential!
>>
>>>> I assume this behaviour is something you just expected to work but then
>>>> didn't.  With your patch it now works as you expected.  So it's kind of
>>>> a bugfix, rather than a change of behaviour the user needs to learn about.
>>
>> To those with similar background or experience it may appear that it should be
>> supported, but hasn't been until now.
>>
>> It is definitely not expected behaviour, given how regedit, reg, etc. expect
>> only hive paths, and how the the current regtool --help reads, clearly expecting
>> Windows style backslash separated registry paths, probably pasted within single
>> quotes. That expectation is changed somewhat by the forward slash sentence.
>> Further changes to expectation needs more documentation.
>>
>>>> The above text is, IMHO, more confusing than helpful to a user just
>>>> asking for regtool --help.  I'd just drop it.
>>
>> It needs documented because it can not in any way be inferred from the existing
>> regtool ---help, and would not be expected, that it should work. It was never
>> previously supported or seen as helpful or necessary, so it should be seen as a
>> non-obvious "surprising" addition, in the opposite sense to "least surprise".
>>
>> Please someone suggest better wording for the help, as that is the only
>> documentation available, and is needed, to update existing and inform new users.
>> Like the code, I tried to maintain the style of the existing help.
>>
>> As an alternative, how about:
>> "To support path completion, a keyname prefix of /proc/registry{,32,64}/ is
>> ignored."
>
> Ok, we can add something to the help text, but the text still sounds
> confusing, even the altenative one.  I think the reason is the negative
> expression "ignore" here.  Why not express this in a positive way like
> this:
>
>   "Use the /proc/registry{,32,64}/ registry path prefix to utilize path
>    completion."
>
> Something like that anyway.

Maybe something may be misinterpreted from your consideration of International
English wording that is not even considered in my native English; "is ignored"
is passive voice but not negative in English, and neither does it appear to be
so in Deutsch (via Google): "Zur Unterstützung der Pfadvervollständigung wird
das Schlüsselnamenpräfix /proc/registry{,32,64}/ ignoriert."
Please advise if you can think why there is a wording issue.

I found the doc/utils.xml entry and added the improved sentence to both,
changing the example to be consistent and the better choice to exemplify the
alternative, and better fit the UG, man pages, and --help.

Please review the resubmission.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: allow /proc/registry{,32,64}/ registry path prefix

Corinna Vinschen-2
In reply to this post by Brian Inglis
Hi Brian,

On Nov 11 10:29, Brian Inglis wrote:
> The user can supply the registry path prefix /proc/registry{,32,64}/ to
> use path completion.

The git commit message does not outline why you're changing the example,

Given that the example doesn't use /proc/registry anyway, what's the
reasoning?  This should either be a patch on its own or at least this
should be mentioned in the commit message.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: Ignore /proc/registry{,32,64}/ prefix, with forward or backslashes, allowing path completion

Corinna Vinschen-2
In reply to this post by Brian Inglis
On Nov 11 13:47, Brian Inglis wrote:

> On 2019-11-11 09:28, Corinna Vinschen wrote:
> > Ok, we can add something to the help text, but the text still sounds
> > confusing, even the altenative one.  I think the reason is the negative
> > expression "ignore" here.  Why not express this in a positive way like
> > this:
> >
> >   "Use the /proc/registry{,32,64}/ registry path prefix to utilize path
> >    completion."
> >
> > Something like that anyway.
>
> Maybe something may be misinterpreted from your consideration of International
> English wording that is not even considered in my native English; "is ignored"
> is passive voice but not negative in English, and neither does it appear to be
> so in Deutsch (via Google): "Zur Unterstützung der Pfadvervollständigung wird
> das Schlüsselnamenpräfix /proc/registry{,32,64}/ ignoriert."
Probably I phrased this wrong.  I was not talking about negative
connotation, but about using a negating expression.  Not doing something
vs. doing the other.   To a non-developer it may be pretty unclear
what's the deal with "ignoring a path prefix".  I think a simpler,
positive (non-negating) expression may be clearer, that's all.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: allow /proc/registry{,32,64}/ registry path prefix

Corinna Vinschen-2
In reply to this post by Corinna Vinschen-2
On Nov 13 09:46, Corinna Vinschen wrote:

> Hi Brian,
>
> On Nov 11 10:29, Brian Inglis wrote:
> > The user can supply the registry path prefix /proc/registry{,32,64}/ to
> > use path completion.
>
> The git commit message does not outline why you're changing the example,
>
> Given that the example doesn't use /proc/registry anyway, what's the
> reasoning?  This should either be a patch on its own or at least this
> should be mentioned in the commit message.
Sigh, I accidentally pushed this patch as is.  Never mind then.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] regtool: allow /proc/registry{,32,64}/ registry path prefix

Brian Inglis
On 2019-11-13 02:38, Corinna Vinschen wrote:
> On Nov 13 09:46, Corinna Vinschen wrote:
>> On Nov 11 10:29, Brian Inglis wrote:
>>> The user can supply the registry path prefix /proc/registry{,32,64}/ to
>>> use path completion.
>> The git commit message does not outline why you're changing the example,
>> Given that the example doesn't use /proc/registry anyway, what's the
>> reasoning?  This should either be a patch on its own or at least this
>> should be mentioned in the commit message.

I explained in my earlier reply that it showed forward slashes and fit the doc
pages better; adding /proc/registry/... would be difficult to fit in the width!

> Sigh, I accidentally pushed this patch as is.  Never mind then.

In my earlier reply I said something which could be added to the commit message
with an --amend, if you wish:

Change doc example to be consistent and a better choice to show
forward slashes, and fit the width of the docs.

New COMMIT_MSG:
regtool: allow /proc/registry{,32,64}/ registry path prefix

The user can supply the registry path prefix /proc/registry{,32,64}/
to use path completion.
Change doc example to be consistent and a better choice to show
forward slashes, and fit the width of the docs.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.