742170 - CGCreateInterfaceObjectsMethod should probably use CGThings for the CreateInterfaceObjects calls.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

Instead of manually munging whitespace.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 2

[Peter Van der Beken [:peterv]]

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 [:bzbarsky]]

> 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 [:bzbarsky]]

Attachment: With that change

Comment 5

[Peter Van der Beken [:peterv]]

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 [:bzbarsky]]

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 [:bzbarsky]]

> 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 [:bzbarsky]]

Per discussion with peter, I'll just smack this with stripTrailingWhitespace().

Comment 9

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/projects/birch/rev/5ccd428446a9

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/5ccd428446a9