Last Comment Bug 767546 - WebIDLError should take a list of locations
: WebIDLError should take a list of locations
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla16
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
Depends on: 756258
Blocks: ParisBindings
  Show dependency treegraph
 
Reported: 2012-06-22 13:24 PDT by Boris Zbarsky [:bz] (Out June 25-July 6)
Modified: 2012-06-30 12:42 PDT (History)
2 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Make the WebIDLError constructor take a list of locations. (41.97 KB, patch)
2012-06-25 10:43 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
justin.lebar+bug: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] (Out June 25-July 6) 2012-06-22 13:24:06 PDT
Right now we pass one location directly and the rest in a list.  That's kinda hacky; we should just pass a list.

Will wait for Peter to land unions before doing this...
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-06-25 10:43:05 PDT
Created attachment 636387 [details] [diff] [review]
Make the WebIDLError constructor take a list of locations.
Comment 2 Justin Lebar (not reading bugmail) 2012-06-29 06:26:21 PDT
> class WebIDLError(Exception):
>-    def __init__(self, message, location, warning=False, extraLocations=[]):
>+    def __init__(self, message, locations, warning=False):
>         self.message = message
>-        self.location = location
>+        self.locations = [str(loc) for loc in locations]
>         self.warning = warning
>-        self.extraLocations = [str(loc) for loc in extraLocations]

Hm, this may have been a good opportunity to use Python's *arg syntax:

  def __init__(self, message, *locations):

There may be a way to shim the warning=False arg into there (although I don't see how that would work), but we could also have WebIDLError and WebIDLWarning as separate classes.

OTOH the list is totally clear, and I don't think we have to mess with this further unless you want to.

> class BuiltinLocation(object):
>     def __init__(self, text):
>-        self.msg = text
>+        self.msg = text + "\n"

To make sure I understood why you did this: You want BuiltinLocation to have an extra blank line to match Location's _pointerline marker?
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-06-29 06:59:33 PDT
> You want BuiltinLocation to have an extra blank line to match Location's _pointerline
> marker?

Hmm.  I added it because stuff was ending up on the same line to my eyes, but yes, looks like that's the main effect.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-06-29 22:54:57 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f44dce601af6
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-06-30 12:42:02 PDT
https://hg.mozilla.org/mozilla-central/rev/f44dce601af6

Note You need to log in before you can comment on or make changes to this bug.