[PATCH] cli: show screen_name when abrt asks for reporters

[Nikola Pajkovsky]

Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
---
 src/cli/report.c |   46 +++++++++++++++++++++++++---------------------
 1 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/src/cli/report.c b/src/cli/report.c
index f00310e..f7af970 100644
--- a/src/cli/report.c
+++ b/src/cli/report.c
@@ -772,36 +772,40 @@ int report(const char *dump_dir_name, int flags)
         for (GList *li = report_events; li; li = li->next)
         {
             char *reporter_name = (char *) li->data;
+            event_config_t *config = get_event_config(reporter_name);
+            if (!config)
+            {
+                log(_("No configuration file found for '%s' reporter,
skipping..."),
+                    reporter_name);
+                continue;
+            }
+
             char question[255];
-            snprintf(question, sizeof(question), _("Report using %s?"),
reporter_name);
+            snprintf(question, sizeof(question), _("Report using %s?"),
+                     (config->screen_name) ? config->screen_name : reporter_name);
             if (!ask_yesno(question))
             {
                 puts(_("Skipping..."));
                 continue;
             }
 
-//TODO: rethink how we associate report events with configs
-            event_config_t *config = get_event_config(reporter_name);
-            if (config)
+            /* TODO: npajkovs; not implemented yet */
+            //const char *rating_required =
get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
+            //if (rating_required
+            //    && string_to_bool(rating_required) == true
+            if (rating < 3)
             {
-                /* TODO: npajkovs; not implemented yet */
-                //const char *rating_required =
get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
-                //if (rating_required
-                //    && string_to_bool(rating_required) == true
-                if (rating < 3)
-                {
-                    puts(_("Reporting disabled because the backtrace is
unusable"));
-
-                    const char *package = get_crash_item_content_or_NULL(crash_data,
FILENAME_PACKAGE);
-                    if (package && package[0])
-                        printf(_("Please try to install debuginfo manually using the
command: \"debuginfo-install %s\" and try again\n"), package);
-
-                    plugins++;
-                    errors++;
-                    continue;
-                }
-                ask_for_missing_settings(reporter_name);
+                puts(_("Reporting disabled because the backtrace is
unusable"));
+
+                const char *package = get_crash_item_content_or_NULL(crash_data,
FILENAME_PACKAGE);
+                if (package && package[0])
+                    printf(_("Please try to install debuginfo manually using the
command: \"debuginfo-install %s\" and try again\n"), package);
+
+                plugins++;
+                errors++;
+                continue;
             }
+            ask_for_missing_settings(reporter_name);
 
             /*
              * to avoid creating list with one item, we probably should
-- 
1.7.1

[Denys Vlasenko]

On Thu, 2011-03-31 at 14:52 +0200, Nikola Pajkovsky wrote:
>  Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
>  ---
>   src/cli/report.c |   46 +++++++++++++++++++++++++---------------------
>   1 files changed, 25 insertions(+), 21 deletions(-)
>  
>  diff --git a/src/cli/report.c b/src/cli/report.c
>  index f00310e..f7af970 100644
>  --- a/src/cli/report.c
>  +++ b/src/cli/report.c
>  @@ -772,36 +772,40 @@ int report(const char *dump_dir_name, int flags)
>           for (GList *li = report_events; li; li = li->next)
>           {
>               char *reporter_name = (char *) li->data;
>  +            event_config_t *config = get_event_config(reporter_name);
>  +            if (!config)
>  +            {
>  +                log(_("No configuration file found for '%s' reporter,
> skipping..."),
>  +                    reporter_name);
>  +                continue;
>  +            } 
This message should be VERB1, and "continue" needs to be dropped.
It's not a requirement for reporter to have configuration - reporter
which needs no configuration is a perfectly valid situation.

(Note: since this way, the message becomes debug message, you need to
remove _() around it)


>               char question[255];
>  -            snprintf(question, sizeof(question), _("Report using %s?"),
> reporter_name);
>  +            snprintf(question, sizeof(question), _("Report using %s?"),
>  +                     (config->screen_name) ? config->screen_name :
> reporter_name);
>               if (!ask_yesno(question))
>               {
>                   puts(_("Skipping..."));
>                   continue;
>               } 
This question should be before if (!config)... check.

>   
>  -//TODO: rethink how we associate report events with configs
>  -            event_config_t *config = get_event_config(reporter_name);
>  -            if (config)
>  +            /* TODO: npajkovs; not implemented yet */
>  +            //const char *rating_required =
> get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
>  +            //if (rating_required
>  +            //    && string_to_bool(rating_required) == true
>  +            if (rating < 3)
>               {
>  -                /* TODO: npajkovs; not implemented yet */
>  -                //const char *rating_required =
> get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
>  -                //if (rating_required
>  -                //    && string_to_bool(rating_required) == true
>  -                if (rating < 3)
>  -                {
>  -                    puts(_("Reporting disabled because the backtrace is
> unusable"));
>  -
>  -                    const char *package = get_crash_item_content_or_NULL(crash_data,
> FILENAME_PACKAGE);
>  -                    if (package && package[0])
>  -                        printf(_("Please try to install debuginfo manually using
> the command: \"debuginfo-install %s\" and try again\n"), package);
>  -
>  -                    plugins++;
>  -                    errors++;
>  -                    continue;
>  -                }
>  -                ask_for_missing_settings(reporter_name);
>  +                puts(_("Reporting disabled because the backtrace is
> unusable"));
>  +
>  +                const char *package = get_crash_item_content_or_NULL(crash_data,
> FILENAME_PACKAGE);
>  +                if (package && package[0])
>  +                    printf(_("Please try to install debuginfo manually using the
> command: \"debuginfo-install %s\" and try again\n"), package);
>  +
>  +                plugins++;
>  +                errors++;
>  +                continue;
>               }
>  +            ask_for_missing_settings(reporter_name);
>   
>               /*
>                * to avoid creating list with one item, we probably should 
-- 
vda

[Nikola Pajkovsky]

Denys Vlasenko <dvlasenk(a)redhat.com&gt; writes:

>  On Thu, 2011-03-31 at 14:52 +0200, Nikola Pajkovsky wrote:
> > Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
> > ---
> >  src/cli/report.c |   46 +++++++++++++++++++++++++---------------------
> >  1 files changed, 25 insertions(+), 21 deletions(-)
> > 
> > diff --git a/src/cli/report.c b/src/cli/report.c
> > index f00310e..f7af970 100644
> > --- a/src/cli/report.c
> > +++ b/src/cli/report.c
> > @@ -772,36 +772,40 @@ int report(const char *dump_dir_name, int flags)
> >          for (GList *li = report_events; li; li = li->next)
> >          {
> >              char *reporter_name = (char *) li->data;
> > +            event_config_t *config = get_event_config(reporter_name);
> > +            if (!config)
> > +            {
> > +                log(_("No configuration file found for '%s' reporter,
> skipping..."),
> > +                    reporter_name);
> > +                continue;
> > +            }
>
>  This message should be VERB1, and "continue" needs to be dropped.
>  It's not a requirement for reporter to have configuration - reporter
>  which needs no configuration is a perfectly valid situation.
>
>  (Note: since this way, the message becomes debug message, you need to
>  remove _() around it) 
Ok, I'll send another patch for that changes.

> >              char question[255];
> > -            snprintf(question, sizeof(question), _("Report using %s?"),
> reporter_name);
> > +            snprintf(question, sizeof(question), _("Report using %s?"),
> > +                     (config->screen_name) ? config->screen_name :
> reporter_name);
> >              if (!ask_yesno(question))
> >              {
> >                  puts(_("Skipping..."));
> >                  continue;
> >              }
>
>  This question should be before if (!config)... check.
>
> >  
> > -//TODO: rethink how we associate report events with configs
> > -            event_config_t *config = get_event_config(reporter_name);
> > -            if (config)
> > +            /* TODO: npajkovs; not implemented yet */
> > +            //const char *rating_required =
> get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
> > +            //if (rating_required
> > +            //    && string_to_bool(rating_required) == true
> > +            if (rating < 3)
> >              {
> > -                /* TODO: npajkovs; not implemented yet */
> > -                //const char *rating_required =
> get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
> > -                //if (rating_required
> > -                //    && string_to_bool(rating_required) == true
> > -                if (rating < 3)
> > -                {
> > -                    puts(_("Reporting disabled because the backtrace is
> unusable"));
> > -
> > -                    const char *package = get_crash_item_content_or_NULL(crash_data,
> FILENAME_PACKAGE);
> > -                    if (package && package[0])
> > -                        printf(_("Please try to install debuginfo manually
> using the command: \"debuginfo-install %s\" and try again\n"), package);
> > -
> > -                    plugins++;
> > -                    errors++;
> > -                    continue;
> > -                }
> > -                ask_for_missing_settings(reporter_name);
> > +                puts(_("Reporting disabled because the backtrace is
> unusable"));
> > +
> > +                const char *package = get_crash_item_content_or_NULL(crash_data,
> FILENAME_PACKAGE);
> > +                if (package && package[0])
> > +                    printf(_("Please try to install debuginfo manually using
> the command: \"debuginfo-install %s\" and try again\n"), package);
> > +
> > +                plugins++;
> > +                errors++;
> > +                continue;
> >              }
> > +            ask_for_missing_settings(reporter_name);
> >  
> >              /*
> >               * to avoid creating list with one item, we probably should 
-- 
Nikola

[PATCHv2] cli: show screen_name when abrt asks for reporters

[Nikola Pajkovsky]

Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
---
 src/cli/report.c |   48 +++++++++++++++++++++++++++---------------------
 1 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/src/cli/report.c b/src/cli/report.c
index f00310e..bbbe80d 100644
--- a/src/cli/report.c
+++ b/src/cli/report.c
@@ -772,36 +772,42 @@ int report(const char *dump_dir_name, int flags)
         for (GList *li = report_events; li; li = li->next)
         {
             char *reporter_name = (char *) li->data;
+            event_config_t *config = get_event_config(reporter_name);
             char question[255];
-            snprintf(question, sizeof(question), _("Report using %s?"),
reporter_name);
+            char *show_reporter_name;
+            if (config)
+                show_reporter_name = (config->screen_name) ? config->screen_name :
reporter_name;
+            else
+                show_reporter_name = reporter_name;
+
+            snprintf(question, sizeof(question), _("Report using %s?"),
show_reporter_name);
+
+            if (!config)
+                VERB1 log("No configuration file found for '%s'
reporter", reporter_name);
+
             if (!ask_yesno(question))
             {
                 puts(_("Skipping..."));
                 continue;
             }
 
-//TODO: rethink how we associate report events with configs
-            event_config_t *config = get_event_config(reporter_name);
-            if (config)
+            /* TODO: npajkovs; not implemented yet */
+            //const char *rating_required =
get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
+            //if (rating_required
+            //    && string_to_bool(rating_required) == true
+            if (rating < 3)
             {
-                /* TODO: npajkovs; not implemented yet */
-                //const char *rating_required =
get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
-                //if (rating_required
-                //    && string_to_bool(rating_required) == true
-                if (rating < 3)
-                {
-                    puts(_("Reporting disabled because the backtrace is
unusable"));
-
-                    const char *package = get_crash_item_content_or_NULL(crash_data,
FILENAME_PACKAGE);
-                    if (package && package[0])
-                        printf(_("Please try to install debuginfo manually using the
command: \"debuginfo-install %s\" and try again\n"), package);
-
-                    plugins++;
-                    errors++;
-                    continue;
-                }
-                ask_for_missing_settings(reporter_name);
+                puts(_("Reporting disabled because the backtrace is
unusable"));
+
+                const char *package = get_crash_item_content_or_NULL(crash_data,
FILENAME_PACKAGE);
+                if (package && package[0])
+                    printf(_("Please try to install debuginfo manually using the
command: \"debuginfo-install %s\" and try again\n"), package);
+
+                plugins++;
+                errors++;
+                continue;
             }
+            ask_for_missing_settings(reporter_name);
 
             /*
              * to avoid creating list with one item, we probably should
-- 
1.7.1

[PATCHv2] cli: show screen_name when abrt asks for reporters

[Denys Vlasenko]

On Mon, 2011-04-04 at 08:53 +0200, Nikola Pajkovsky wrote:
>  Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
>  ---
>   src/cli/report.c |   48 +++++++++++++++++++++++++++---------------------
>   1 files changed, 27 insertions(+), 21 deletions(-)
>  
>  diff --git a/src/cli/report.c b/src/cli/report.c
>  index f00310e..bbbe80d 100644
>  --- a/src/cli/report.c
>  +++ b/src/cli/report.c
>  @@ -772,36 +772,42 @@ int report(const char *dump_dir_name, int flags)
>           for (GList *li = report_events; li; li = li->next)
>           {
>               char *reporter_name = (char *) li->data;
>  +            event_config_t *config = get_event_config(reporter_name);
>               char question[255];
>  -            snprintf(question, sizeof(question), _("Report using %s?"),
> reporter_name);
>  +            char *show_reporter_name;
>  +            if (config)
>  +                show_reporter_name = (config->screen_name) ? config->screen_name :
> reporter_name;
>  +            else
>  +                show_reporter_name = reporter_name;
>  +
>  +            snprintf(question, sizeof(question), _("Report using %s?"),
> show_reporter_name);
>  +
>  +            if (!config)
>  +                VERB1 log("No configuration file found for '%s'
> reporter", reporter_name);
>  +
>               if (!ask_yesno(question))
>               {
>                   puts(_("Skipping..."));
>                   continue;
>               }
>   
>  -//TODO: rethink how we associate report events with configs
>  -            event_config_t *config = get_event_config(reporter_name);
>  -            if (config)
>  +            /* TODO: npajkovs; not implemented yet */
>  +            //const char *rating_required =
> get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
>  +            //if (rating_required
>  +            //    && string_to_bool(rating_required) == true
>  +            if (rating < 3)
>               {
>  -                /* TODO: npajkovs; not implemented yet */
>  -                //const char *rating_required =
> get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
>  -                //if (rating_required
>  -                //    && string_to_bool(rating_required) == true
>  -                if (rating < 3)
>  -                {
>  -                    puts(_("Reporting disabled because the backtrace is
> unusable"));
>  -
>  -                    const char *package = get_crash_item_content_or_NULL(crash_data,
> FILENAME_PACKAGE);
>  -                    if (package && package[0])
>  -                        printf(_("Please try to install debuginfo manually using
> the command: \"debuginfo-install %s\" and try again\n"), package);
>  -
>  -                    plugins++;
>  -                    errors++;
>  -                    continue;
>  -                }
>  -                ask_for_missing_settings(reporter_name);
>  +                puts(_("Reporting disabled because the backtrace is
> unusable"));
>  +
>  +                const char *package = get_crash_item_content_or_NULL(crash_data,
> FILENAME_PACKAGE);
>  +                if (package && package[0])
>  +                    printf(_("Please try to install debuginfo manually using the
> command: \"debuginfo-install %s\" and try again\n"), package);
>  +
>  +                plugins++;
>  +                errors++;
>  +                continue;
>               }
>  +            ask_for_missing_settings(reporter_name);
>   
>               /*
>                * to avoid creating list with one item, we probably should 
Looks good to me.
-- 
vda