Last Comment Bug 742170 - CGCreateInterfaceObjectsMethod should probably use CGThings for the CreateInterfaceObjects calls.
: CGCreateInterfaceObjectsMethod should probably use CGThings for the CreateInt...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Mac OS X
: P2 normal (vote)
: mozilla15
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
Depends on:
Blocks: ParisBindings 742217
  Show dependency treegraph
 
Reported: 2012-04-03 20:51 PDT by Boris Zbarsky [:bz] (Out June 25-July 6)
Modified: 2012-04-24 17:59 PDT (History)
2 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Like so (5.79 KB, patch)
2012-04-09 12:59 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
peterv: review+
Details | Diff | Review
With that change (7.03 KB, patch)
2012-04-10 09:34 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
peterv: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-03 20:51:27 PDT
Instead of manually munging whitespace.
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-09 12:59:18 PDT
Created attachment 613362 [details] [diff] [review]
Like so
Comment 2 Peter Van der Beken [:peterv] 2012-04-10 05:05:29 PDT
Comment on attachment 613362 [details] [diff] [review]
Like so

Review of attachment 613362 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/Codegen.py
@@ +824,2 @@
>              if len(idsToInit) > 1:
> +                initIds = CGWrapper(initIds, pre="(", post=")", reindent=True)

I wonder if it'd make sense to add support for this to CGList (pre and post for len > 1).

@@ +863,5 @@
> +        ourList = CGList([CGGeneric(getParentProto)], "\n\n")
> +        if initIds is not None:
> +            ourList.prepend(initIds)
> +        if chrome is not None:
> +            ourList.append(chrome)

Would it make sense to make CGList skip None values?
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-10 09:20:31 PDT
> I wonder if it'd make sense to add support for this to CGList (pre and post for len > 1).

We'd need to add the reindent thing there too...  We could do that, I guess.  Want me to?

> Would it make sense to make CGList skip None values?

Yes, absolutely.  I'll do that instead.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-10 09:34:38 PDT
Created attachment 613644 [details] [diff] [review]
With that change
Comment 5 Peter Van der Beken [:peterv] 2012-04-18 07:02:08 PDT
Comment on attachment 613644 [details] [diff] [review]
With that change

Review of attachment 613644 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/Codegen.py
@@ +251,4 @@
>      def declare(self):
> +        decl = self.child.declare()
> +        if self.reindent:
> +            decl = decl.replace("\n", "\n" + (" " * len(self.declarePre)))

Should these .rstrip(" ") to avoid trailing whitespace?
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-18 07:31:53 PDT
Hmm.  Long-term, we should never have trailing whitespace in cgitems, imo.

But you raise another interesting question.  We don't really want to add whitespace to blank lines.  So perhaps this should do something more like what CGIndenter does, in terms of the regexp?
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-18 07:32:21 PDT
> Long-term, we should never have trailing whitespace in cgitems, imo.

That includes trailing newlines, which are the case rstrip() would help with.
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-18 07:41:42 PDT
Per discussion with peter, I'll just smack this with stripTrailingWhitespace().
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-18 09:14:56 PDT
https://hg.mozilla.org/projects/birch/rev/5ccd428446a9
Comment 10 :Ehsan Akhgari (busy, don't ask for review please) 2012-04-24 17:59:17 PDT
https://hg.mozilla.org/mozilla-central/rev/5ccd428446a9

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