Fwd: Delivery Status Notification (Failure)
Commit Message
On Sun, Nov 23, 2014 at 11:18 AM, Doug Evans <xdje42@gmail.com> wrote:
> [+ keiths, since this is linespec related, and I know
> how much he loves linespecs! :-)]
>
> On Thu, Nov 20, 2014 at 6:57 AM, David Taylor
> <dtaylor@usendtaylorx2l.lss.emc.com> wrote:
>> Doug Evans <xdje42@gmail.com> wrote:
>>> The "," in "-at LOCATION," seems a bit random, relative to other commands.
>>>
>>> Maybe it is the best way to go.
>>> If so, I'd like to see the reasons why it exists documented in the code.
>>>
>>> Can we just remove the , and require -- when necessary?
>>
>> It marks the end of the location and the start of the macro. It is
>> patterned after
>>
>> maint agent [-at location,] EXPRESSION
>> and
>> maint agent-eval [-at location,] EXPRESSION
>>
>> I did it the same way for consistency.
>
> Ah. Can't fault that. :-)
>
> Still, I'm more ok with u/i visible hacks in maint commands
> than with normal commands.
> Bubbling this up into non-maint commands means needing
> to now worry about consistency with all the other commands.
>
>> It seems unnecessary, but the location parsing code stops at comma but
>> not at space. Presumably that is so that file names can contain spaces.
>> But, that is just a guess on my part. I personally dislike file names
>> containing spaces.
>
> Such names need to be quoted.
> OTOH c++ functions with parameters, for example,
> don't (usually) need to be quoted.
>
> linespec.c has this:
> static const char * const linespec_keywords[] = { "if", "thread", "task" };
>
> This may not be the best solution, and it might involve a few more
> changes to linespec.c (or elsewhere) but that's one alternative:
> static const char * const linespec_keywords[] = { "if", "thread",
> "task", "--" };
>
> It works, but "--" wasn't intended for this purpose (mark the
> end of the linespec), so it's just a not-well-thought-out-idea.
>
> I tried to think of a situation where using a comma here would
> lead to u/i warts later, but couldn't.
> E.g., if the comma gets used in other contexts
> then will users start complaining that "b foo, thread 1" doesn't work?
> If that day comes we *could* allow the comma in that context.
> But it would be odd that a comma is *required* in some
> contexts and not in others.
>
> In the end, I'm ok with the patch, but
> I think the docs (both offline and online) need to highlight the comma
> as being a requirement for "-at" since it's not intuitive.
> Plus I think we need to document in some linespec-related
> place that "," is now required to be recognized as ending a linespec.
> I don't know if that exists today.
> [FAOD, a "," within a linespec is still ok,
> e.g., "info macro -at foo(int, int), BAR"]
> Ok with those changes.
Re: Plus I think we need to document in some linespec-related
place that "," is now required to be recognized as ending a linespec.
FAOD, I was thinking of, say, adding a comment to the
function-comment for decode_line_full in linespec.h.
E.g.,
or some such.
The point being to make the comma terminator
being part of the spec of what's a valid linespec.
---
IWBN to "fix" the linespec parsing machinery to recognize
the end of the line spec so that the comma wasn't required.
I don't want to make that a prerequisite for this patch,
but down the road if that happens then the comma
could just become optional.
@@ -133,7 +133,11 @@ extern struct symtabs_and_lines
entry describing all the matching locations. If FILTER is
non-NULL, then only locations whose canonical name is equal (in the
strcmp sense) to FILTER will be returned; all others will be
- filtered out. */
+ filtered out.
+
+ Note for new callers: If you need a way to mark the end of the linespec
+ in *ARGPTR, fyi the "info macro -at LOCATION, MACRO" command uses a comma.
+ A comma within the linespec is still ok (e.g., "foo (int, int)"). */
extern void decode_line_full (char **argptr, int flags,
struct symtab *default_symtab, int default_line,