patchPSPP - Patches: patch #6405, Make multiple error messages...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #6405: Make multiple error messages display in a single dialog

Submitted by:  Ben Pfaff <blp>
Submitted on:  Wed 06 Feb 2008 04:49:06 AM UTC  
 
Category:  None Item Group:  None
Status:  Done Assigned to:  None
Open/Closed:  Closed

Sun 10 Feb 2008 07:09:23 AM UTC, comment #5: 

I committed this, with John's recommended changes.

Thanks!

Ben Pfaff <blp>
Project Administrator
Wed 06 Feb 2008 11:53:20 PM UTC, comment #4: 

> Let's do it the way you've written it.


OK.  I am perfectly willing to revisit this in the future.

Ben Pfaff <blp>
Project Administrator
Wed 06 Feb 2008 11:19:07 PM UTC, comment #3: 

OK.  Let's do it the way you've written it.

In the long term, I think we need to look at the whole messaging issue.  But this is certainly an improvement.

John Darrington <jmd>
Project Administrator
Wed 06 Feb 2008 04:29:57 PM UTC, comment #2: 

Wow, I'm glad you're paying attention.  I'm going to apply all of your suggestions, except that I have a few follow-up comments:

>The condition of this if statement is always false, since
>get_widget_assert, asserts that its return value is non NULL


Well, sort of.  I actually tested this path, and if there's no widget named "message", it just to use_fallback.  get_widget_assert doesn't assert, really, it calls g_critical(), which by default just prints a message.

>Why do we have to drop any of the messages? They're in a scrolled
>window, so the can all be viewed easily. If there's a huge number,
>then we could have two GtkTextArea widgets in a GtkvPaned, one
>defaulting to the top, the other to the bottom of the textbuffer.


The problem is that the amount of memory that could be consumed that way is unlimited.  We support reading data files of unbounded size.  If each input line produces an error, and we store each of those errors in memory, then we will eventually run out of memory.

If there's a good way to display the contents of a text file of unbounded size, we could use a file.

Ben Pfaff <blp>
Project Administrator
Wed 06 Feb 2008 11:59:41 AM UTC, comment #1: 

In message-dialog.c, use g_warning in place of the first fputs.

In code such as :

  text_view = GTK_TEXT_VIEW (get_widget_assert (xml, "message"));
  if (text_view == NULL)
    goto use_fallback;

The condition of this if statement is always false, since get_widget_assert, asserts that its return value is non NULL

As you've called XML_NEW, you need to unref the xml object. Rather than doing this within popup_messages, I suggest that you  call it once, and pass a pointer to the GtkDialog, in which case, you shouldn't destroy the dialog at the end of the function.
I don't think that gtk_widget_show_all is necessary either.

Can you give the dialog an appropriate window title.  Should the modal property be set?

Set the "shadow-type" property of scrolled window to "Etched In".

Set the "editable" propery of the text area to false.

Why do we have to drop any of the messages?  They're in a scrolled window, so the can all be viewed easily.  If there's a huge number, then we could have two GtkTextArea widgets in a GtkvPaned, one defaulting to the top,  the other to the bottom of the textbuffer.

 
John Darrington <jmd>
Project Administrator
Wed 06 Feb 2008 04:49:06 AM UTC, original submission:  

Currently, each error message is displayed in a separate dialog box.  This is not a good fit for the upcoming text-import patch, which can easily cause hundreds of thousands of errors if a field is designated with the wrong format type.

This patch causes multiple error messages to be displayed in a scrolling text field within a single dialog box.  When more than 110 messages are issued, it drops all but the first 100 and last 10 of them, which are usually the ones of interest in my experience.

In earlier discussion with JMD, we also considered the alternative of making the error log a permanent window, or perhaps a pane within another window.  I haven't tried out those possibilities yet.

Comments?

Ben Pfaff <blp>
Project Administrator

 

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jmd (Posted a comment)
  • -email is unavailable- added by blp (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-02-10 blp StatusReady For Test/Review => Done
        Open/ClosedOpen => Closed
    2008-02-06 blp Attached File- => Added message-dialog-use-textview.patch, #14967

    Back to the top


    Powered by Savane 3.5