Prompt user to report bugs

Message ID 20140514110742.GA25550@blade.nx
State Committed
Headers

Commit Message

Gary Benson May 14, 2014, 11:07 a.m. UTC
  Hi all,

This patch prints a couple of extra lines in internal_vproblem
to prompt the user to report the bug that's just happened.

Ok to commit?

Thanks,
Gary
  

Comments

Mark Kettenis May 14, 2014, 11:22 a.m. UTC | #1
> Date: Wed, 14 May 2014 12:07:42 +0100
> From: Gary Benson <gbenson@redhat.com>
> 
> Hi all,
> 
> This patch prints a couple of extra lines in internal_vproblem
> to prompt the user to report the bug that's just happened.
> 
> Ok to commit?

Can't hurt.

> -- 
> 2014-05-14  Gary Benson  <gbenson@redhat.com>
> 
> 	* utils.c (internal_vproblem): Prompt the user to file a report.
> 
> diff --git a/gdb/utils.c b/gdb/utils.c
> index a8a7cb3..905ce6d 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -746,6 +746,14 @@ internal_vproblem (struct internal_problem *problem,
>    else
>      internal_error (__FILE__, __LINE__, _("bad switch"));
>  
> +  fputs_unfiltered (_("\nThis is a bug, please report it."), gdb_stderr);
> +  if (REPORT_BUGS_TO[0])
> +    {
> +      fputs_unfiltered (_("  For instructions, see:\n"), gdb_stderr);
> +      fprintf_unfiltered (gdb_stderr, _("%s."), REPORT_BUGS_TO);
> +    }
> +  fputs_unfiltered ("\n\n", gdb_stderr);
> +
>    if (problem->should_dump_core == internal_problem_ask)
>      {
>        if (!can_dump_core (reason))
> 
>
  
Yao Qi May 14, 2014, 12:36 p.m. UTC | #2
On 05/14/2014 07:07 PM, Gary Benson wrote:
> This patch prints a couple of extra lines in internal_vproblem
> to prompt the user to report the bug that's just happened.

GDB prints that when it starts.

$ ./gdb
GNU gdb (GDB) 7.7.50.20140514-cvs
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Do we want to print the same message again in internal error message?
  
Gary Benson May 14, 2014, 1:15 p.m. UTC | #3
Yao Qi wrote:
> On 05/14/2014 07:07 PM, Gary Benson wrote:
> > This patch prints a couple of extra lines in internal_vproblem
> > to prompt the user to report the bug that's just happened.
> 
> GDB prints that when it starts.
> 
> $ ./gdb
> GNU gdb (GDB) 7.7.50.20140514-cvs
> Copyright (C) 2014 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "i686-pc-linux-gnu".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Do we want to print the same message again in internal error
> message?

Yes, I think so.  The internal error could occur many thousands
of lines after the startup message is printed.

Thanks,
Gary
  
Tom Tromey May 14, 2014, 8:14 p.m. UTC | #4
>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> Ok to commit?

Gary> 2014-05-14  Gary Benson  <gbenson@redhat.com>
Gary> 	* utils.c (internal_vproblem): Prompt the user to file a report.

Please give people a few days to comment on this.
It's the kind of thing which usually generates comments.

So far the feedback seems good though.  And the patch itself is ok.
So barring objections feel free to put it in next week.

thanks,
Tom
  
Doug Evans May 14, 2014, 9:05 p.m. UTC | #5
On Wed, May 14, 2014 at 4:07 AM, Gary Benson <gbenson@redhat.com> wrote:
> Hi all,
>
> This patch prints a couple of extra lines in internal_vproblem
> to prompt the user to report the bug that's just happened.
>
> Ok to commit?
>
> Thanks,
> Gary
>
> --
> 2014-05-14  Gary Benson  <gbenson@redhat.com>
>
>         * utils.c (internal_vproblem): Prompt the user to file a report.
>
> diff --git a/gdb/utils.c b/gdb/utils.c
> index a8a7cb3..905ce6d 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -746,6 +746,14 @@ internal_vproblem (struct internal_problem *problem,
>    else
>      internal_error (__FILE__, __LINE__, _("bad switch"));
>
> +  fputs_unfiltered (_("\nThis is a bug, please report it."), gdb_stderr);
> +  if (REPORT_BUGS_TO[0])
> +    {
> +      fputs_unfiltered (_("  For instructions, see:\n"), gdb_stderr);
> +      fprintf_unfiltered (gdb_stderr, _("%s."), REPORT_BUGS_TO);
> +    }
> +  fputs_unfiltered ("\n\n", gdb_stderr);
> +
>    if (problem->should_dump_core == internal_problem_ask)
>      {
>        if (!can_dump_core (reason))
>

top.c doesn't wrap "%s." in _().
I don't know if it matters, it's just odd to see "%s." wrapped.
  
Paul_Koning@Dell.com May 14, 2014, 9:11 p.m. UTC | #6
On May 14, 2014, at 5:05 PM, Doug Evans <dje@google.com> wrote:

> On Wed, May 14, 2014 at 4:07 AM, Gary Benson <gbenson@redhat.com> wrote:
>> Hi all,
>> 
>> This patch prints a couple of extra lines in internal_vproblem
>> to prompt the user to report the bug that's just happened.
>> 
>> Ok to commit?
>> 
>> Thanks,
>> Gary
>> 
>> --
>> 2014-05-14  Gary Benson  <gbenson@redhat.com>
>> 
>>        * utils.c (internal_vproblem): Prompt the user to file a report.
>> 
>> diff --git a/gdb/utils.c b/gdb/utils.c
>> index a8a7cb3..905ce6d 100644
>> --- a/gdb/utils.c
>> +++ b/gdb/utils.c
>> @@ -746,6 +746,14 @@ internal_vproblem (struct internal_problem *problem,
>>   else
>>     internal_error (__FILE__, __LINE__, _("bad switch"));
>> 
>> +  fputs_unfiltered (_("\nThis is a bug, please report it."), gdb_stderr);
>> +  if (REPORT_BUGS_TO[0])
>> +    {
>> +      fputs_unfiltered (_("  For instructions, see:\n"), gdb_stderr);
>> +      fprintf_unfiltered (gdb_stderr, _("%s."), REPORT_BUGS_TO);
>> +    }
>> +  fputs_unfiltered ("\n\n", gdb_stderr);
>> +
>>   if (problem->should_dump_core == internal_problem_ask)
>>     {
>>       if (!can_dump_core (reason))
>> 
> 
> top.c doesn't wrap "%s." in _().
> I don't know if it matters, it's just odd to see "%s." wrapped.


It also seems odd to see “%s.” inside _( ) since there’s nothing to translate there.

	paul
  
Doug Evans May 14, 2014, 10:24 p.m. UTC | #7
On Wed, May 14, 2014 at 2:11 PM,  <Paul_Koning@dell.com> wrote:
>
> On May 14, 2014, at 5:05 PM, Doug Evans <dje@google.com> wrote:
>
>> On Wed, May 14, 2014 at 4:07 AM, Gary Benson <gbenson@redhat.com> wrote:
>>> Hi all,
>>>
>>> This patch prints a couple of extra lines in internal_vproblem
>>> to prompt the user to report the bug that's just happened.
>>>
>>> Ok to commit?
>>>
>>> Thanks,
>>> Gary
>>>
>>> --
>>> 2014-05-14  Gary Benson  <gbenson@redhat.com>
>>>
>>>        * utils.c (internal_vproblem): Prompt the user to file a report.
>>>
>>> diff --git a/gdb/utils.c b/gdb/utils.c
>>> index a8a7cb3..905ce6d 100644
>>> --- a/gdb/utils.c
>>> +++ b/gdb/utils.c
>>> @@ -746,6 +746,14 @@ internal_vproblem (struct internal_problem *problem,
>>>   else
>>>     internal_error (__FILE__, __LINE__, _("bad switch"));
>>>
>>> +  fputs_unfiltered (_("\nThis is a bug, please report it."), gdb_stderr);
>>> +  if (REPORT_BUGS_TO[0])
>>> +    {
>>> +      fputs_unfiltered (_("  For instructions, see:\n"), gdb_stderr);
>>> +      fprintf_unfiltered (gdb_stderr, _("%s."), REPORT_BUGS_TO);
>>> +    }
>>> +  fputs_unfiltered ("\n\n", gdb_stderr);
>>> +
>>>   if (problem->should_dump_core == internal_problem_ask)
>>>     {
>>>       if (!can_dump_core (reason))
>>>
>>
>> top.c doesn't wrap "%s." in _().
>> I don't know if it matters, it's just odd to see "%s." wrapped.
>
>
> It also seems odd to see “%s.” inside _( ) since there’s nothing to translate there.

That's why it seemed odd to me too. :-)
[Though what do I know ... maybe someone might want to use something
other than "." at the end.]
  
Pierre Muller May 15, 2014, 10:07 a.m. UTC | #8
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Paul_Koning@Dell.com
> Envoyé : mercredi 14 mai 2014 23:11
> À : dje@google.com
> Cc : gbenson@redhat.com; gdb-patches@sourceware.org; tromey@redhat.com
> Objet : Re: [PATCH] Prompt user to report bugs
> 
> 
> On May 14, 2014, at 5:05 PM, Doug Evans <dje@google.com> wrote:
> 
> > On Wed, May 14, 2014 at 4:07 AM, Gary Benson <gbenson@redhat.com>
> wrote:
> >> Hi all,
> >>
> >> This patch prints a couple of extra lines in internal_vproblem
> >> to prompt the user to report the bug that's just happened.
> >>
> >> Ok to commit?
> >>
> >> Thanks,
> >> Gary
> >>
> >> --
> >> 2014-05-14  Gary Benson  <gbenson@redhat.com>
> >>
> >>        * utils.c (internal_vproblem): Prompt the user to file a
> report.
> >>
> >> diff --git a/gdb/utils.c b/gdb/utils.c
> >> index a8a7cb3..905ce6d 100644
> >> --- a/gdb/utils.c
> >> +++ b/gdb/utils.c
> >> @@ -746,6 +746,14 @@ internal_vproblem (struct internal_problem
> *problem,
> >>   else
> >>     internal_error (__FILE__, __LINE__, _("bad switch"));
> >>
> >> +  fputs_unfiltered (_("\nThis is a bug, please report it."),
> gdb_stderr);
> >> +  if (REPORT_BUGS_TO[0])
> >> +    {
> >> +      fputs_unfiltered (_("  For instructions, see:\n"),
> gdb_stderr);
> >> +      fprintf_unfiltered (gdb_stderr, _("%s."), REPORT_BUGS_TO);
> >> +    }
> >> +  fputs_unfiltered ("\n\n", gdb_stderr);
> >> +
> >>   if (problem->should_dump_core == internal_problem_ask)
> >>     {
> >>       if (!can_dump_core (reason))
> >>
> >
> > top.c doesn't wrap "%s." in _().
> > I don't know if it matters, it's just odd to see "%s." wrapped.
> 
> 
> It also seems odd to see “%s.” inside _( ) since there’s nothing to
> translate there.

  I suspect that the main reason is that
otherwise you would generate a new ARI warning
about missing _()...

  Of course, you are right that _() around "%s." is useless,
but this, on the other hand seems to indicate that,
while the "  For instructions, see: "
might become " Pour les instructions, voir : " in French,
the next part REPORT_BUGS_TO
will not get translated at all...

  OK, presently this is a simple URL link...
but we might choose one day to do localized versions of that bug report page
so why not simply use

+  fputs_unfiltered (_("\nThis is a bug, please report it"), gdb_stderr);
+  if (REPORT_BUGS_TO[0])
+    {
+      fputs_unfiltered (_(".  For instructions, see:\n"), gdb_stderr);
+      fprintf_unfiltered (gdb_stderr, _(REPORT_BUGS_TO));
+    }
+  fputs_unfiltered (".\n\n", gdb_stderr);

 To allow for possible localization of the URL page?

Pierre Muller
  
Gary Benson May 15, 2014, 11:37 a.m. UTC | #9
Pierre Muller wrote:
> so why not simply use
> 
> +  fputs_unfiltered (_("\nThis is a bug, please report it"), gdb_stderr);
> +  if (REPORT_BUGS_TO[0])
> +    {
> +      fputs_unfiltered (_(".  For instructions, see:\n"), gdb_stderr);
> +      fprintf_unfiltered (gdb_stderr, _(REPORT_BUGS_TO));
> +    }
> +  fputs_unfiltered (".\n\n", gdb_stderr);
> 
>  To allow for possible localization of the URL page?

Did you mean:
> +      fprintf_unfiltered (gdb_stderr, "%s.", _(REPORT_BUGS_TO));
                                         ^^^^^^

The code you posted would break if _(REPORT_BUGS_TO) had a "%" in it.

Thanks,
Gary
  
Gary Benson May 15, 2014, 11:39 a.m. UTC | #10
Doug Evans wrote:
> On Wed, May 14, 2014 at 2:11 PM,  <Paul_Koning@dell.com> wrote:
> > On May 14, 2014, at 5:05 PM, Doug Evans <dje@google.com> wrote:
> > > On Wed, May 14, 2014 at 4:07 AM, Gary Benson <gbenson@redhat.com> wrote:
> > > > 2014-05-14  Gary Benson  <gbenson@redhat.com>
> > > >
> > > >        * utils.c (internal_vproblem): Prompt the user to file a report.
> > > >
> > > > diff --git a/gdb/utils.c b/gdb/utils.c
> > > > index a8a7cb3..905ce6d 100644
> > > > --- a/gdb/utils.c
> > > > +++ b/gdb/utils.c
> > > > @@ -746,6 +746,14 @@ internal_vproblem (struct internal_problem *problem,
> > > >   else
> > > >     internal_error (__FILE__, __LINE__, _("bad switch"));
> > > >
> > > > +  fputs_unfiltered (_("\nThis is a bug, please report it."), gdb_stderr);
> > > > +  if (REPORT_BUGS_TO[0])
> > > > +    {
> > > > +      fputs_unfiltered (_("  For instructions, see:\n"), gdb_stderr);
> > > > +      fprintf_unfiltered (gdb_stderr, _("%s."), REPORT_BUGS_TO);
> > > > +    }
> > > > +  fputs_unfiltered ("\n\n", gdb_stderr);
> > > > +
> > > >   if (problem->should_dump_core == internal_problem_ask)
> > > >     {
> > > >       if (!can_dump_core (reason))
> > > >
> > >
> > > top.c doesn't wrap "%s." in _().
> > > I don't know if it matters, it's just odd to see "%s." wrapped.
> >
> > It also seems odd to see “%s.” inside _( ) since there’s nothing
> > to translate there.
> 
> That's why it seemed odd to me too. :-)
> [Though what do I know ... maybe someone might want to use something
> other than "." at the end.]

Japanese has a different sentence terminator:
http://en.wikipedia.org/wiki/Japanese_punctuation#Full_stop
Possibly other languages do to.

I'm easy either way, if people want it wrapped I'll wrap it,
if people want it bare I can do that too :)

Cheers,
Gary
  
Doug Evans May 15, 2014, 5:10 p.m. UTC | #11
On Thu, May 15, 2014 at 4:39 AM, Gary Benson <gbenson@redhat.com> wrote:
> Doug Evans wrote:
>> On Wed, May 14, 2014 at 2:11 PM,  <Paul_Koning@dell.com> wrote:
>> > On May 14, 2014, at 5:05 PM, Doug Evans <dje@google.com> wrote:
>> > > On Wed, May 14, 2014 at 4:07 AM, Gary Benson <gbenson@redhat.com> wrote:
>> > > > 2014-05-14  Gary Benson  <gbenson@redhat.com>
>> > > >
>> > > >        * utils.c (internal_vproblem): Prompt the user to file a report.
>> > > >
>> > > > diff --git a/gdb/utils.c b/gdb/utils.c
>> > > > index a8a7cb3..905ce6d 100644
>> > > > --- a/gdb/utils.c
>> > > > +++ b/gdb/utils.c
>> > > > @@ -746,6 +746,14 @@ internal_vproblem (struct internal_problem *problem,
>> > > >   else
>> > > >     internal_error (__FILE__, __LINE__, _("bad switch"));
>> > > >
>> > > > +  fputs_unfiltered (_("\nThis is a bug, please report it."), gdb_stderr);
>> > > > +  if (REPORT_BUGS_TO[0])
>> > > > +    {
>> > > > +      fputs_unfiltered (_("  For instructions, see:\n"), gdb_stderr);
>> > > > +      fprintf_unfiltered (gdb_stderr, _("%s."), REPORT_BUGS_TO);
>> > > > +    }
>> > > > +  fputs_unfiltered ("\n\n", gdb_stderr);
>> > > > +
>> > > >   if (problem->should_dump_core == internal_problem_ask)
>> > > >     {
>> > > >       if (!can_dump_core (reason))
>> > > >
>> > >
>> > > top.c doesn't wrap "%s." in _().
>> > > I don't know if it matters, it's just odd to see "%s." wrapped.
>> >
>> > It also seems odd to see “%s.” inside _( ) since there’s nothing
>> > to translate there.
>>
>> That's why it seemed odd to me too. :-)
>> [Though what do I know ... maybe someone might want to use something
>> other than "." at the end.]
>
> Japanese has a different sentence terminator:
> http://en.wikipedia.org/wiki/Japanese_punctuation#Full_stop
> Possibly other languages do to.

Heh.  I didn't know that.
Though I was thinking of Chantho when I wrote that. 1/2 :-)
http://tardis.wikia.com/wiki/Chantho

Chan <http://www.gnu.org/software/gdb/bugs/> tho ?

[The Chan would probably go before "For instructions, see", but why
ruin a good (bad?) joke. :)]

> I'm easy either way, if people want it wrapped I'll wrap it,
> if people want it bare I can do that too :)

I don't have a strong preference, other than to be consistent.
If it'd be a protocol violation to change top.c at the same time,
and if no one really cares, then to keep things simple
I'd say stick with what top.c does.
  

Patch

diff --git a/gdb/utils.c b/gdb/utils.c
index a8a7cb3..905ce6d 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -746,6 +746,14 @@  internal_vproblem (struct internal_problem *problem,
   else
     internal_error (__FILE__, __LINE__, _("bad switch"));
 
+  fputs_unfiltered (_("\nThis is a bug, please report it."), gdb_stderr);
+  if (REPORT_BUGS_TO[0])
+    {
+      fputs_unfiltered (_("  For instructions, see:\n"), gdb_stderr);
+      fprintf_unfiltered (gdb_stderr, _("%s."), REPORT_BUGS_TO);
+    }
+  fputs_unfiltered ("\n\n", gdb_stderr);
+
   if (problem->should_dump_core == internal_problem_ask)
     {
       if (!can_dump_core (reason))