Factor the JS shell's repl into a few functions

RESOLVED FIXED in mozilla24

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

unspecified
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Assignee)

Description

5 years ago
Who knows, maybe if the code is readable enough someone will take bug 480198. :)
(Assignee)

Comment 1

5 years ago
Created attachment 759597 [details] [diff] [review]
Part 1 - Factor out ReadEvalPrintLoop, v1

This comes in five patches, all of them pretty trivial.
Assignee: general → jorendorff
Attachment #759597 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 2

5 years ago
Created attachment 759598 [details] [diff] [review]
Part 2 - Factor out RunFile, v1
Attachment #759598 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 3

5 years ago
Created attachment 759599 [details] [diff] [review]
Part 3 - Declare at initialization
Attachment #759599 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 4

5 years ago
Created attachment 759600 [details] [diff] [review]
Part 4 - Use a Vector<char> for the input buffer (instead of manual malloc/free)
Attachment #759600 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 5

5 years ago
Created attachment 759601 [details] [diff] [review]
Part 5 - Use CompileOptions, and Handles, and a few other changes
Attachment #759601 - Flags: review?(jwalden+bmo)
Comment on attachment 759597 [details] [diff] [review]
Part 1 - Factor out ReadEvalPrintLoop, v1

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

I don't care where the style nits get fixed in this patch stack, just fix them by the end.  :-)  I'm noting 'em as I see 'em.

::: js/src/shell/js.cpp
@@ +528,5 @@
>      free(buffer);
>      fprintf(gOutFile, "\n");
> +}
> +
> +class AutoCloseInputFile {

{ on new line.

@@ +533,5 @@
> +  private:
> +    FILE *f_;
> +  public:
> +    explicit AutoCloseInputFile(FILE *f) : f_(f) {}
> +    ~AutoCloseInputFile() { if (f_ && f_ != stdin) fclose(f_); }

I'd rather see this in multiple lines, honestly.

@@ +566,5 @@
> +         * hack, and gobble the first line if it starts with '#'.
> +         */
> +        int ch = fgetc(file);
> +        if (ch == '#') {
> +            while((ch = fgetc(file)) != EOF) {

Space after while.
Attachment #759597 - Flags: review?(jwalden+bmo) → review+
Attachment #759598 - Flags: review?(jwalden+bmo) → review+
Attachment #759599 - Flags: review?(jwalden+bmo) → review+
Attachment #759600 - Flags: review?(jwalden+bmo) → review+
Attachment #759601 - Flags: review?(jwalden+bmo) → review+
You need to log in before you can comment on or make changes to this bug.