Summary: | Fix the bytecode code generator scripts to pretty print BytecodeStructs.h and BytecodeIndices.h. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||
Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | fpizlo, keith_miller, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer, ysuzuki | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Mark Lam
2018-11-30 17:30:18 PST
Created attachment 356271 [details]
proposed patch.
Comment on attachment 356271 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=356271&action=review r=me > Source/JavaScriptCore/generator/Opcode.rb:208 > +#{@metadata.struct(self)}#{@metadata.accessor} Shouldn’t there be a new line between these? > Source/JavaScriptCore/generator/Opcode.rb:214 > +#{print_members(" ", &:field)}#{@metadata.field(" ")} Ditto. Comment on attachment 356271 [details]
proposed patch.
Can you do this in a way that doesn't make the ruby code worse to read? Why not just do some post processing on the string?
(In reply to Saam Barati from comment #3) > Comment on attachment 356271 [details] > proposed patch. > > Can you do this in a way that doesn't make the ruby code worse to read? Why > not just do some post processing on the string? I think this would also lead to fewer LOC changed Comment on attachment 356271 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=356271&action=review Thanks for the review. >> Source/JavaScriptCore/generator/Opcode.rb:208 >> +#{@metadata.struct(self)}#{@metadata.accessor} > > Shouldn’t there be a new line between these? This is needed because the #{@metadata.struct(self)} maybe an empty string, and we don't want the extra '\n'. >> Source/JavaScriptCore/generator/Opcode.rb:214 >> +#{print_members(" ", &:field)}#{@metadata.field(" ")} > > Ditto. Same here. #{print_members(" ", &:field)} may be an empty string and we don't want the extra '\n'. (In reply to Saam Barati from comment #4) > (In reply to Saam Barati from comment #3) > > Comment on attachment 356271 [details] > > proposed patch. > > > > Can you do this in a way that doesn't make the ruby code worse to read? Why > > not just do some post processing on the string? > > I think this would also lead to fewer LOC changed Perhaps, but my initial thinking is that post-processing would require a parser of sorts that recognizes when to indent and unindent. I don't think it's trivial in the general case, but perhaps we could implement mini post-processors that might be doable. I'm going to leave this patch as is for now, and move on. I think we'll be working with the ruby scripts less than we will with the generated C++ file. I'm going to land it so that we get the benefit of a properly formatted C++ file, and revisit the formatting of the ruby script at a later date if we find it to be a detriment to our productivity. Landed in r238804: <http://trac.webkit.org/r238804>. |