1472101 - [BinAST] Fix some styling in BinAST auto-generated code.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tooru Fujisawa [:arai]]

Before updating the BinAST code to match the latest spec, I'd like to apply some style fix.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Part 0: Add a script to build auto-generated BinAST code.

Just to make it possible to update auto-generated files easily, added a script to do cargo run.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Remove some duplicate empty lines in auto-generated BinAST code.

Instead of always put newline, added WrapLine trait with wrap_line function,
which appends newline if the given string is not empty.
so that we won't have duplicate empty lines.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Add a space after switch in auto-generated BinAST code.

Just added a missing space.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8988669 [details] [diff] [review]
Part 1: Remove some duplicate empty lines in auto-generated BinAST code.

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

Thanks for the patch. Just some tiny nitpicking :)

::: js/src/frontend/binsource/src/main.rs
@@ +19,5 @@
>  
>  use itertools::Itertools;
>  
> +/// A string or string-like construction that can be wrapped.
> +trait WrapLine {

Is that really "wrapping"?

Also, I suspect that you should rather patch `ToStr::newline` in binjs_meta/src/util.rs, no?

Comment 5

[Tooru Fujisawa [:arai]]

Thank you for reviewing :D

(In reply to David Teller [:Yoric] (please use "needinfo") from comment #4)
> > +/// A string or string-like construction that can be wrapped.
> > +trait WrapLine {
> 
> Is that really "wrapping"?

Yeah, newline or newline_if_SOMETHING would be better.


> Also, I suspect that you should rather patch `ToStr::newline` in
> binjs_meta/src/util.rs, no?

I'll patch it later, and then switch to use it.

Comment 6

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1fcfc8b1be986b3b7c5d86e0514ab0b16df5d607
Bug 1472101 - Part 0: Add a script to build auto-generated BinAST code. r=Yoric

https://hg.mozilla.org/integration/mozilla-inbound/rev/fc7abf84687b599ec3168f4aac4bb8087eb9f620
Bug 1472101 - Part 1: Remove some duplicate empty lines in auto-generated BinAST code. r=Yoric

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb57020766137aa6f654fdc38fd2f96b9181dd01
Bug 1472101 - Part 2: Add a space after switch in auto-generated BinAST code. r=Yoric

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/1fcfc8b1be98
https://hg.mozilla.org/mozilla-central/rev/fc7abf84687b
https://hg.mozilla.org/mozilla-central/rev/fb5702076613