Last Comment Bug 685025 - Shell should be able to read from stdin
: Shell should be able to read from stdin
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla9
Assigned To: Chris Leary [:cdleary] (not checking bugmail)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-06 16:28 PDT by Chris Leary [:cdleary] (not checking bugmail)
Modified: 2011-09-15 07:39 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Solo dash argument. (1.67 KB, patch)
2011-09-06 16:28 PDT, Chris Leary [:cdleary] (not checking bugmail)
brendan: review+
Details | Diff | Splinter Review

Description Chris Leary [:cdleary] (not checking bugmail) 2011-09-06 16:28:03 PDT
Created attachment 558657 [details] [diff] [review]
Solo dash argument.

$ ./js - < test2.js 
Hoo!
$ ./js -f test.js - < test2.js 
Whoo
Hoo!
Comment 1 Brendan Eich [:brendan] 2011-09-06 22:50:47 PDT
Comment on attachment 558657 [details] [diff] [review]
Solo dash argument.

># HG changeset patch
># Parent 6a4e5dbe0d64d7646306b4c9fc50a69b29631551
>
>diff --git a/js/src/shell/jsoptparse.cpp b/js/src/shell/jsoptparse.cpp
>--- a/js/src/shell/jsoptparse.cpp
>+++ b/js/src/shell/jsoptparse.cpp
>@@ -373,19 +373,21 @@ OptionParser::parseArgs(int inputArgc, c
>     /* Permit a "no more options" capability, like |--| offers in many shell interfaces. */
>     bool optionsAllowed = true;
> 
>     for (size_t i = 1; i < argc; ++i) {
>         char *arg = argv[i];
>         Result r;
>         if (arg[0] == '-' && optionsAllowed) {
>             /* Option. */
>-            size_t arglen = strlen(arg);
>-            if (arglen < 2) /* Do not permit solo dash option. */
>-                return error("Invalid dash option");
>+            if (arg[1] == '\0') {
>+                /* Solo dash option is actually a 'stdin' argument. */
>+                r = handleArg(argc, argv, &i, &optionsAllowed);

Note statement immediately above.

>+                goto have_result;
>+            }
> 
>             Option *opt;
>             if (arg[1] == '-') {
>                 /* Long option. */
>                 opt = findOption(arg + 2);
>                 if (!opt)
>                     return error("Invalid long option: %s", arg);
>             } else {
>@@ -397,16 +399,17 @@ OptionParser::parseArgs(int inputArgc, c
>                     return error("Invalid short option: %s", arg);
>             }
> 
>             r = handleOption(opt, argc, argv, &i, &optionsAllowed);
>         } else {
>             /* Argument. */
>             r = handleArg(argc, argv, &i, &optionsAllowed);

Same as this basic block.

>         }
>+  have_result:

Succeeded by what's at this label, so instead of duplicating the r = handleArg(...) call and using a goto, how about:

         if (arg[0] == '-' && arg[1] != '\0' && optionsAllowed) {

up front, and unify the handleArg call (and avoid the strlen < 2 test).

r=me with that. Thanks,

/be

Note You need to log in before you can comment on or make changes to this bug.