#11602: install_scripts should use "$@" instead of $*
------------------------------+---------------------------------------------
Reporter: Stefan | Owner: jason
Type: defect | Status: needs_review
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:
------------------------------+---------------------------------------------
Comment(by leif):
Replying to [comment:19 jhpalmieri]:
> 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.
I'm almost happy with the delta patch, should test the new version though.
[[BR]]
> > (Also, `"/usr/local/bin"` should perhaps be double-''back''quoted.)
> I quoted it both ways, so it's a backquoted string.
Just double-backquotes (monospaced font) would have sufficed.
[[BR]]
> I made a few other changes: [...]
> 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.
Well, the notion of shortcuts (or better ''shortcut scripts'') in the
description / docstring isn't all bad, as it emphasizes their purpose and
perhaps connotes their "light-weightness".
The docstring actually lacks a description of what a shortcut [script] is
or really / technically does; an example saying that the scripts make e.g.
`gp` an alias for `sage -gp`, which starts the stand-alone PARI/GP
interpreter, would be helpful.
When talking about ''software components'', I'd use their ''name'' rather
than the corresponding command, i.e. GAP, Maxima, Singular, PARI/GP,
MWrank (?), Mercurial, GNU R etc., without quotes. In contrast, when
referring to the commands / options to `sage`, I'd typeset them as such
(with double-backticks), but also omit single-quotes visible in the HTML
documentation.
(I guess William just copied and pasted the contents of the Python list.)
I'd also typeset `root` and `sage` (lower-case) monospaced in
''"You may need to run Sage as root in order to ..."''
and perhaps mention `sudo` or `sudo sage -c
"install_scripts(<directory>)"`, or combine it with the next sentence,
giving the example
{{{
#!sh
sudo sage -c "install_scripts('/usr/local/bin')"
}}}
(Do you want to add a "shortcut script" `install_scripts` to
`$SAGE_ROOT/local/bin/` or `$SAGE_ROOT/`? :P )
[[BR]]
We should check once in advance that the given directory is writable by
the user, right after the presence test, in order to give a nice(r) error
message [earlier].
Btw., I still get an ugly traceback if the directory doesn't exist; can we
suppress that or do we really have to `raise`? (I'd prefer just printing
an error message and `return`.)
[[BR]]
I thought we would advise the user (at the end, if any scripts were
created) to add `directory` to his/her `PATH` in case it isn't already
there.
----
(As you may have noticed, I've meanwhile tested also the v2 a little while
writing...)
If you don't want to make further changes, I won't mind and give it
positive review as is (unless I missed some severe flaw, which I don't
expect).
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11602#comment:22>
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.
