998080 - Codegen.py: don't require every definition_body() to return indented code

Status: RESOLVED FIXED

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

Description

[Jason Orendorff [:jorendorff]]

In bug 986492 comment 20, peterv wrote:
> >      def _define(self, fromDeclare=False):
> > +        return self.definition_prologue(fromDeclare) + self.definition_body() + self.definition_epilogue()
> 
> I wonder if it wouldn't make more sense to just indent
> self.definition_body() here, instead of in all the implementations.

Yes, I think so. It was a pain indenting it everywhere.

Caveat: There are few function bodies that contain goto labels, which shouldn't be indented. I think the solution for those is a heuristic hack. But that is a matter of taste.

Comment 1

[Jason Orendorff [:jorendorff]]

Along the same lines: CGEventClass's three methods impl{Traverse,Unlink,Trace} all contain manual indentation, but it would be prettier for the caller to do it using fill().

Comment 2

[Boris Zbarsky [:bzbarsky]]

> There are few function bodies that contain goto labels

Code using goto in codegen will get an automatic r-.  We have no such code and shouldn't be adding any.

Comment 3

[Jason Orendorff [:jorendorff]]

Oh, I guess I was thinking of switch statements. This is trivial then.

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: Part 1 - Make CGMethodCall not indent

In three parts. Each part focuses on a single method name, changes all implementations of that method to produce non-indented output, and changes its callers to indent instead.

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: Part 2 - Make generate_code not indent

Comment 6

[Jason Orendorff [:jorendorff]]

Attachment: Part 3 - Make definition_body not indent

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 8418410 [details] [diff] [review]
Part 3 - Make definition_body not indent

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

Thanks for doing this! I wonder if collapsing some of these patches would allow us to preserve blame (where you're adding things and then removing them in subsequent patches).

Comment 8

[Jason Orendorff [:jorendorff]]

Oh, good point. I'll qfold them to land.

Comment 9

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82231f3042ee

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/82231f3042ee