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

Submitter:  Ben Pfaff <blp>
Submitted:  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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

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)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    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.13-4448.
    Corresponding source code