#11602: install_scripts should use "$@" instead of $*
------------------------------+---------------------------------------------
Reporter: Stefan | Owner: jason
Type: defect | Status: needs_work
Priority: minor | Milestone: sage-4.7.2
Component: misc | Keywords: install_scripts, hg, command
line
Work_issues: | Upstream: N/A
Reviewer: Leif Leonhardy | Author: John Palmieri
Merged: | Dependencies:
------------------------------+---------------------------------------------
Changes (by jhpalmieri):
* status: positive_review => needs_work
Comment:
Replying to [comment:18 leif]:
> Only a few minor things:
I've attached [attachment:trac_11602-install-scripts.v2.patch] to deal
with these; [attachment:trac_11602-delta.patch] shows the differences
between the two versions.
> * Somehow the empty line is removed from the docstring when I type
`install_scripts?`:
Maybe the `.. note::` markup can't handle more than one paragraph, or our
sphinxified text version of the docstring can't. Anyway, I broke it into
two notes.
> (Also, `"/usr/local/bin"` should perhaps be double-''back''quoted.)
I quoted it both ways, so it's a backquoted string.
> * You could add yourself to `AUTHORS`. :-)
Okay.
> * I'd quote the `directory` name in the last message.
Okay
> (Also, rerunning the command may make sense if a newer version of Sage
ships more components, or the user later installs some optional component.
Similarly, a system-wide version might vanish later if it gets uninstalled
or the `PATH` changes.)
I think it's best to leave this as is. I can't think of a way to phrase
it which clarifies things and which isn't too complicated. We could add a
phrase like "All things being equal" at the beginning, but I think it's
best unchanged.
> * I personally don't like raising `RuntimeError`s for rather "expected"
error conditions; I thought there was a way to at least suppress the
traceback, but I currently don't recall how. (I did get ''"`RuntimeError`
doesn't take keyword arguments"'' when I tried... ;-) ) [[BR]]
I think `OSError` is better. That's more consistent with the behavior of
the `os.path` module, for example.
> (There also could be a ''"Creating script ..."'' message in advance,
since an error in `o.open()` or `o.write()` could otherwise be associated
with ''"Checking that Sage has the command ..."''.)
Okay.
> * An `else:` is superfluous whenever the `if`-branch ends with e.g.
`continue`, but that's partially a matter of taste. (Omitting it here
appears more readable to me, but I don't mind.)
I omitted it.
I made a few other changes: it now checks to see if any scripts were
written at all. If not, then the ending message doesn't say "Finished
creating scripts", etc. Also, the original docstring (and the function
name itself) talked about "scripts", while all of the messages printed
during execution talked about "shortcuts". I changed it so now they all
talk about "scripts" only.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11602#comment:19>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
