public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Doug Evans <xdje42@gmail.com>
To: David Taylor <dtaylor@usendtaylorx2l.lss.emc.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
	Eli Zaretskii <eliz@gnu.org>, 	Keith Seitz <keiths@redhat.com>
Subject: Re: Fwd: Delivery Status Notification (Failure)
Date: Sun, 23 Nov 2014 19:18:00 -0000	[thread overview]
Message-ID: <CAP9bCMS+7MD+5+nuVa-AtvH4Zwr4Y751SWGMZHXpXQKr6uXfdA@mail.gmail.com> (raw)
In-Reply-To: <12971.1416495464@usendtaylorx2l>

[+ 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.

  reply	other threads:[~2014-11-23 19:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-10 21:41 RFA: info macro [-at LOCATION,] David Taylor
2014-11-11  3:44 ` Eli Zaretskii
2014-11-11 14:01   ` David Taylor
2014-11-11 15:41     ` Eli Zaretskii
2014-11-19 21:00   ` RFA info macro [-at LOCATION,] (v2) David Taylor
2014-11-19 21:20     ` Eli Zaretskii
2014-11-20  2:38     ` Doug Evans
     [not found]       ` <001a1132e59ca4575e0508413955@google.com>
     [not found]         ` <CAP9bCMRJr4Fbunbnt-93FYnWUgDqjaLWZ731_rZp-JP8qkKf=w@mail.gmail.com>
2014-11-20 14:58           ` Fwd: Delivery Status Notification (Failure) David Taylor
2014-11-23 19:18             ` Doug Evans [this message]
2014-11-23 20:00               ` Doug Evans

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAP9bCMS+7MD+5+nuVa-AtvH4Zwr4Y751SWGMZHXpXQKr6uXfdA@mail.gmail.com \
    --to=xdje42@gmail.com \
    --cc=dtaylor@usendtaylorx2l.lss.emc.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=keiths@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).