Re: [PATCH v3 3/5] Introduce gdb_tilde_expand

[Pedro Alves <palves@redhat.com>]

On 09/22/2017 07:07 PM, Sergio Durigan Junior wrote:
> On Friday, September 22 2017, Pedro Alves wrote:
> 
>>>>> +/* See common/gdb_tilde_expand.h.  */
>>>>> +
>>>>> +std::string
>>>>> +gdb_tilde_expand (const char *dir)
>>>>> +{
>>>>> +  gdb_glob glob (dir, GLOB_TILDE | GLOB_TILDE_CHECK | GLOB_ONLYDIR, NULL);
>>>>
>>>> By my reading of man glob, GLOB_TILDE_CHECK already implies GLOB_TILDE.
>>>
>>> Yes, but I think it pays to be explicit in this case.
>>
>> I think it only adds to confusion.  I did "man glob",
>> saw that GLOB_TILDE_CHECK implies GLOB_TILDE and then
>> got to wonder why is GLOB_TILDE being passed explicitly.
> 
> Well, I personally don't really see the reason for the confusion here
> since you could have read GLOB_TILDE's entry as well and noticed that
> there is nothing fancy being done here, but I respect your way of
> thinking, so I will just remove GLOB_TILDE_CHECK.

I did not suggest to remove GLOB_TILDE_CHECK.

What I'm saying is that the man page looks like this:

      GLOB_TILDE
              Carry out tilde expansion. (...)

      GLOB_TILDE_CHECK
              This provides behavior similar to that of GLOB_TILDE.  The difference is that (...)

And from reading this, my interpretation is that GLOB_TILDE
and GLOB_TILDE_CHECK are two different modes.  From that
description, I'd think it reasonable for glob to reject
"GLOB_TILDE | GLOB_TILDE_CHECK" as ambiguous -- which mode
to you want with that?

Note that it says "provides behavior similar", not
"in combination with GLOB_TILDE".

With GLOB_TILDE | GLOB_TILDE_CHECK, I assumed that you
meant GLOB_TILDE_CHECK, but I have no idea whether that's
what all glob implementations actually end up interpreting
it as.  But why leave that ambiguity?  Just specify what
you want, exactly.

Thanks,
Pedro Alves