Closed Bug 987618 Opened 10 years ago Closed 10 years ago

More minor cosmetic changes in Codegen.py

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: jorendorff, Assigned: jorendorff)

References

Details

Attachments

(1 file)

Trivial, but this one should be a very quick review.

- Usual python style is to skip the parens around an
  unpacking (that is, destructuring) assignment target.

- Python has nicer syntax for:
    "not x in y" -> "x not in y"
    "not x is y" -> "x is not y"
  Same behavior.

- Sorting a list of tuples by the first element is as simple as sorted(mylist).
  (Tuple comparison is what you'd expect.)

and other stuff.
This is the last one.
Attachment #8396331 - Flags: review?(bzbarsky)
Depends on: 986492
Blocks: 987007
Comment on attachment 8396331 [details] [diff] [review]
bug-987618-Codegen-cosmetic-v1.patch

>- Usual python style is to skip the parens around an
>  unpacking (that is, destructuring) assignment target.

:(  That seems much harder to read to me... Ah, well.

>+            memberString, itemCount = defineMembers(self, memberList,
>                                                       itemCount, separator)

Fix indent?

>-        cgThings = CGList((CGIndenter(t, declareOnly=True) for t in cgThings), "\n")
>+        cgThings = CGList([CGIndenter(t, declareOnly=True) for t in cgThings], "\n")

CGList will do a list() already, so passing a generator to it is fine.  Why this change?

The rest looks fine.
Attachment #8396331 - Flags: review?(bzbarsky) → review+
> >+            memberString, itemCount = defineMembers(self, memberList,
> >                                                       itemCount, separator)
> 
> Fix indent?

Done.

> >-        cgThings = CGList((CGIndenter(t, declareOnly=True) for t in cgThings), "\n")
> >+        cgThings = CGList([CGIndenter(t, declareOnly=True) for t in cgThings], "\n")
> 
> CGList will do a list() already, so passing a generator to it is fine.  Why
> this change?

Oh! Reverted.
https://hg.mozilla.org/mozilla-central/rev/cfe59efce491
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.