Closed Bug 1337465 Opened 7 years ago Closed 7 years ago

JS shell namespace objects have poor help strings

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: sfink, Assigned: sfink)

Details

Attachments

(1 file)

js> help(os.file)
os.file - interface object
  os.file.readFile os.file.readRelativeToScript os.file.writeTypedArrayToFile os.file.redirect os.file.redirectErr os.file.close
I'm not sure who to use for my shell-related reviews. So I'll pick on you by default.

With this patch:

js> help(os.file)
os.file - interface object
  readFile(filename, ["binary"])
  readRelativeToScript(filename, ["binary"])
  writeTypedArrayToFile(filename, data)
  redirect([path-or-object])
  redirectErr([path-or-object])
  close(object)

I've long had plans to split up the huge pile of shell functions into namespaces. It'll probably never happen and should perhaps be replaced with spidernode anyway, but I'll still incrementally move that way when I have the chance.
Attachment #8834519 - Flags: review?(jcoppeard)
Comment on attachment 8834519 [details] [diff] [review]
Create proper help strings for JS shell namespace objects

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

::: js/src/shell/jsshell.cpp
@@ +102,1 @@
>      return true;

Should this return false if the specified property wasn't found?
Attachment #8834519 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #2)
> Comment on attachment 8834519 [details] [diff] [review]
> Create proper help strings for JS shell namespace objects
> 
> Review of attachment 8834519 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/shell/jsshell.cpp
> @@ +102,1 @@
> >      return true;
> 
> Should this return false if the specified property wasn't found?

Oh, good point. I could just assert. But maybe we do oom simulation during startup? Yeah, I'll throw. Thanks.
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d4116cb3c9d
Create proper help strings for JS shell namespace objects, r=jonco
https://hg.mozilla.org/mozilla-central/rev/1d4116cb3c9d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.