gEDA-bug: [Bug 700448] Re: Poor reporting of schematic load errors

[Peter TB Brett <700448@xxxxxxxxxxxxxxxxxx>]

Hi Eivind,

I've now had time to do a more in-depth review of your patch (looking at
the latest commit on your github branch, http://goo.gl/tJW63), and I
have a few comments. I'm afraid I still haven't been able to actually go
through it on a line-by-line basis, though, so there might be some
remaining issues that I've missed. Sorry. :-(

- Please indent with spaces, not tabs.

- In o_complex_prepare_place(), the correct thing to do if the symbol
can't be loaded is to abort the symbol placement operation and show an
error dialog box to the user.

- I personally am not a fan of generic_msg_dialog(), because it usually
doesn't show the correct message dialog variant (warning/error/info etc)
for the type of message to be displayed. Consider using a
GtkMessageDialog directly.

- For the preview, would it be possible not to pop up a modal dialog and
instead show the error message in the preview pane somehow? Displaying a
preview is not a "critical operation" such that failure should interrupt
what the user is doing. By contrast, if the user attempts to open the
file for viewing/editing, that load *is* a critical operation and a
message dialog would be exactly the right thing. What do you think?

- I'm not convinced that EDA_ERROR_READ is the best possible name for
this error. What about EDA_ERROR_PARSE or something that similarly
describes the source of the error more specifically? To me,
EDA_ERROR_READ suggests that an error occurred in actually *reading* the
data from storage/network... It's also not clear why you're using
EDA_ERROR_READ in some places and G_FILE_ERROR_FAILED in others.

- Similarly, you've chosen "invalid-string" as the key symbol for the
corresponding Scheme error. "Invalid" has quite a large number of
interpretations. Consider something like "string-format" or a name that
otherwise specifically identifies the class of error that you're looking
for.

This might be a good opportunity to also standardise the formatting and
wording of the errors emitted during file loading.

Thanks for your hard work on this. I appreciate that the existing file
loader code is both slightly baroque and also spread across a bunch of
different files, and that fixing it up is a non-trivial exercise!

                               Peter

-- 
You received this bug notification because you are a member of gEDA Bug
Team, which is subscribed to gEDA.
https://bugs.launchpad.net/bugs/700448

Title:
  Poor reporting of schematic load errors

Status in GPL Electronic Design Automation tools:
  In Progress

Bug description:
  libgeda doesn't signal an error when a schematic or symbol file
  contains invalid syntax, or when any other load error is encountered.

  For example, gnetlist succeeds even when the input schematics
  specified contain unusable garbage.

  Steps to reproduce:

  ÂÂecho GARBAGE > test.sch;
  ÂÂif gnetlist -ggeda test.sch > /dev/null 2>/dev/null
  ÂÂthen
  ÂÂÂÂecho Succeeded
  ÂÂelse
  ÂÂÂÂecho Failed
  ÂÂfi

  Expected output:

  ÂÂFailed

  Actual output:

  ÂÂSucceeded

  Thanks to Peter Clifton for pointing out this problem.

To manage notifications about this bug go to:
https://bugs.launchpad.net/geda/+bug/700448/+subscriptions


_______________________________________________
geda-bug mailing list
geda-bug@xxxxxxxxxxxxxx
http://www.seul.org/cgi-bin/mailman/listinfo/geda-bug