[patch] More suggestive error_is_running message

[patch] More suggestive error_is_running message

[Jan Kratochvil]

Hi,

I was debugging some testsuite race before I figured out in non-stop mode one
has to wait first for
	[Thread N] #1 stopped.
before executing any commands otherwise one gets:
	Cannot execute this command while the selected thread is running.

Not sure if this error message would help me or not but I found it tricky.

No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.
I am not going to check it in on my own.


Thanks,
Jan


gdb/
2012-04-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Provide more specific error_is_running error message.
	* infrun.c (error_is_running): Extend the error message text.

--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3038,7 +3045,9 @@ void
 error_is_running (void)
 {
   error (_("Cannot execute this command while "
-	   "the selected thread is running."));
+	   "the selected thread is running.  "
+	   "(You may want to use 'interrupt' "
+	   "and wait for '[Thread N] #1 stopped.'.)"));
 }
 
 void

Re: [patch] More suggestive error_is_running message

[Sergio Durigan Junior]

On Tuesday, April 10 2012, Jan Kratochvil wrote:

> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -3038,7 +3045,9 @@ void
>  error_is_running (void)
>  {
>    error (_("Cannot execute this command while "
> -	   "the selected thread is running."));
> +	   "the selected thread is running.  "
> +	   "(You may want to use 'interrupt' "
> +	   "and wait for '[Thread N] #1 stopped.'.)"));
>  }

Sorry for the nitpick: I guess we use `' (as in `interrupt'), and not ''.

-- 
Sergio

Re: [patch] More suggestive error_is_running message

[Jan Kratochvil]

On Tue, 10 Apr 2012 22:52:10 +0200, Sergio Durigan Junior wrote:
> On Tuesday, April 10 2012, Jan Kratochvil wrote:
> > +	   "and wait for '[Thread N] #1 stopped.'.)"));
> 
> Sorry for the nitpick: I guess we use `' (as in `interrupt'), and not ''.

http://www.gnu.org/prep/standards/standards.html#Quote-Characters
	Although GNU programs traditionally used 0x60 (‘`’) for opening and
	0x27 (‘'’) for closing quotes, nowadays quotes ‘`like this'’ are
	typically rendered asymmetrically, so quoting ‘"like this"’ or ‘'like
	this'’ typically looks better. 

I was surprised `this is no longer the standard'.


Thanks,
Jan

Re: [patch] More suggestive error_is_running message

[Sergio Durigan Junior]

On Tuesday, April 10 2012, Jan Kratochvil wrote:

> On Tue, 10 Apr 2012 22:52:10 +0200, Sergio Durigan Junior wrote:
>> On Tuesday, April 10 2012, Jan Kratochvil wrote:
>> > +	   "and wait for '[Thread N] #1 stopped.'.)"));
>> 
>> Sorry for the nitpick: I guess we use `' (as in `interrupt'), and not ''.
>
> http://www.gnu.org/prep/standards/standards.html#Quote-Characters
> 	Although GNU programs traditionally used 0x60 (‘`’) for opening and
> 	0x27 (‘'’) for closing quotes, nowadays quotes ‘`like this'’ are
> 	typically rendered asymmetrically, so quoting ‘"like this"’ or ‘'like
> 	this'’ typically looks better. 
>
> I was surprised `this is no longer the standard'.

In such cases I never know if those new standards apply for new
projects, or for existing projects as well.  If they apply to existing
projects, then I'm not sure there should be any massive
replacement/adjustment in the codebase to reflect those new rules.

Anyway, I was surprised by this new rule as well.  Thanks for pointing
it out.

-- 
Sergio

Re: [patch] More suggestive error_is_running message

[Tom Tromey]

Sergio> In such cases I never know if those new standards apply for new
Sergio> projects, or for existing projects as well.  If they apply to existing
Sergio> projects, then I'm not sure there should be any massive
Sergio> replacement/adjustment in the codebase to reflect those new rules.

I tend to think we should change, but it is a surprisingly tricky area.

Tom

Re: [patch] More suggestive error_is_running message

[Joel Brobecker]

> gdb/
> 2012-04-10  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Provide more specific error_is_running error message.
> 	* infrun.c (error_is_running): Extend the error message text.
> 
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -3038,7 +3045,9 @@ void
>  error_is_running (void)
>  {
>    error (_("Cannot execute this command while "
> -	   "the selected thread is running."));
> +	   "the selected thread is running.  "
> +	   "(You may want to use 'interrupt' "
> +	   "and wait for '[Thread N] #1 stopped.'.)"));
>  }

Just my two cents on this, I don't really have a strong opinion.

I'd put the new output on a second line, and also avoid duplicating
the actual notification. That way, if we change its format, it doesn't
become inconsistent. There's also the case of being in GDB/MI mode, as
well as the case where the ptid is displayed differently on non-Linux
platforms. Similarly, I think we want to avoid the "interrupt" command
name, because I suspect that many users will be using a graphical
frontend and that's not going to be very helpful to them.

Trying to come up with a concise error message, I couldn't really
find something all that great. I think that it's OK to drop the part
that talks about waiting for the stop notification. Once you've used
it once, you'll know, and for the first time, even if you are fast
enough to send the interrupt and emit the command again before the
thread stopped, you'll get the same message, followed by the stop
notification. That should be enough to understand that you had to wait?

So how about:

    Cannot execute this command while the selected thread is running.
    You need to interrupt this thread's execution first.

?

-- 
Joel

[cancel] [patch] More suggestive error_is_running message

[Jan Kratochvil]

On Wed, 11 Apr 2012 17:25:31 +0200, Joel Brobecker wrote:
> >    error (_("Cannot execute this command while "
> > -	   "the selected thread is running."));
> > +	   "the selected thread is running.  "
> > +	   "(You may want to use 'interrupt' "
> > +	   "and wait for '[Thread N] #1 stopped.'.)"));
> >  }
> 
> Just my two cents on this, I don't really have a strong opinion.
> 
> I'd put the new output on a second line,

OK, I see error is already being used with embedded \n.


> I think that it's OK to drop the part that talks about waiting for the stop
> notification. Once you've used it once, you'll know,

The whole point of my patch was not notify about the _wait_, not about the
_interrupt_, as this is what one cannot easily catch when debugging a testcase
after converting it full-stop -> non-stop.  If you try to reproduce the
problem by hand it works.  OK, I admit I am too stupid to write GDB testcases.


Thanks,
Jan

Re: [cancel] [patch] More suggestive error_is_running message

[Joel Brobecker]

> The whole point of my patch was not notify about the _wait_, not about
> the _interrupt_, as this is what one cannot easily catch when
> debugging a testcase after converting it full-stop -> non-stop.  If
> you try to reproduce the problem by hand it works.  OK, I admit I am
> too stupid to write GDB testcases.

I missed a bit the context leading to the improvement you were
suggesting, but it was not my intention to say that you are "too
stupid to [...]" at all. This sounds like a harsh criticism of
yourself - perhaps a quirk of English not being our native language
for both of us.

In any case, as I said, I don't have a strong opinion. I know that
we want to both have concise error messages, while at the same time
making GDB more explanatory on some confusing or tricky  situations.
This is a real conundrum.

-- 
Joel

Re: [cancel] [patch] More suggestive error_is_running message

[Jan Kratochvil]

On Wed, 11 Apr 2012 18:45:22 +0200, Joel Brobecker wrote:
> In any case, as I said, I don't have a strong opinion. I know that
> we want to both have concise error messages, while at the same time
> making GDB more explanatory on some confusing or tricky  situations.
> This is a real conundrum.

In general GDB should always suggest possible solutions to a problem.
Stating just the problem may not be enough for the user.

This is like GDB says just "(no debugging symbols found)" but Fedora GDB says
also:
Missing separate debuginfos, use: debuginfo-install bc-1.06.95-3.fc15.x86_64
considered good in:
http://lists.fedoraproject.org/pipermail/devel/2012-April/165756.html

This is why I tried to suggest (one of) a possible solutions to the problem,
but it somehow did not go in, maintaining all these messages downstream is
expensive.


Regards,
Jan

Re: [cancel] [patch] More suggestive error_is_running message

[Joel Brobecker]

> In general GDB should always suggest possible solutions to a problem.
> Stating just the problem may not be enough for the user.

Agreed on that. I have learnt to appreciate git's helpful hints
when something happens. We just need to find the right wording
to avoid being too verbose and undoing some of the benefits of
the work done earlier to cut on verbosity.  I realize it is
a little disingenous of me to put everything on the same level,
as some of the verbose output that was removed was simply useless
or redundant, but I didn't know how to express it concisely.

Anyway, what I was also trying to say is that my comments where
meant to try to help with the wording. More often than not,
I'll settle for something that works, or is good enough, even
if it's not perfect. We both use English as a second language,
so it's normal that we're having a tougher time writing good
English error messages.

-- 
Joel