Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

command line option parsing #21

Open
williamstein opened this issue Sep 12, 2006 · 72 comments
Open

command line option parsing #21

williamstein opened this issue Sep 12, 2006 · 72 comments

Comments

@williamstein
Copy link
Contributor

We should improve and/or modernize and/or revise Sage's command-line parsing.

Two ideas, which could be debated endlessly and could also be implemented independently of each other:

  • use Python's argparse module to handle the parsing, rather than a shell script
  • change Sage's options from flags to subcommands: sage --package ... would be changed to sage package ..., etc. (comment:56 and comment:57 list some possibilities)

Depends on #9958

CC: @kini @saraedum @gvol

Component: user interface

Issue created by migration from https://trac.sagemath.org/ticket/21

@williamstein
Copy link
Contributor Author

comment:1

no -- you can't combine command line options like that. this isn't a bug
but a not implemented yet issue.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Sep 11, 2007

comment:2

This should be fixable, but the long term goal is to do a proper rewrite of the command line options.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin added this to the sage-2.10 milestone Sep 11, 2007
@garyfurnish garyfurnish mannequin self-assigned this Mar 16, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Sep 24, 2008

comment:5

See also #180 for a bunch of related failures due to the option parsing being dumb :o

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin assigned sagetrac-mabshoff and unassigned garyfurnish Sep 24, 2008
@kcrisman
Copy link
Member

comment:7

Note that sage -bn now builds, then does notebook, though of course it doesn't fix the issue here.

@jhpalmieri
Copy link
Member

Attachment: makefile.gz

the file SAGE_ROOT/makefile

@jhpalmieri
Copy link
Member

Attachment: makefile.diff.gz

Attachment: trac_21-extcode.patch.gz

extcode repo

@jhpalmieri
Copy link
Member

sagenb repo

@jhpalmieri
Copy link
Member

comment:8

Attachment: trac_21-sagenb.patch.gz

Here are patches. After applying "trac_21-scripts.patch", you may need to make "SAGE_ROOT/local/bin/sage-sage.py" executable. The build process works for me with these patches. For the standard packages, the third line in

if [ "$SAGE_LOCAL" = "" ]; then
   echo "SAGE_LOCAL undefined ... exiting";
   echo "Maybe run 'sage -sh'?"
   exit 1
fi

should be changed to "Maybe run 'sage --sh'?", but this doesn't affect the functioning of the packages, and otherwise, they don't need changing. I haven't looked at optional packages.

This approaches uses Python's optparse to parse command-line options. If someone wants to write a version using shflags or some other package, go ahead.

I propose the following approach, whether using these patches or other ones:

  • first, we include new command-line options but don't turn them on by default, instead printing a message like this one when you type "sage [...]" with a nonempty argument:
    Note: Using old-style Sage command-line options. 

    To try out Sage's experimental GNU/Posix-style command-line options 
    (for example, 'sage --notebook' instead of 'sage -notebook'), set the 
    environment variable $SAGE_NEW_OPTIONS to something nonempty. 
    To bypass this message, set the environment variable 
    $SAGE_SKIP_OPTIONS_MESSAGE to something nonempty. 

Running "sage" (with no arguments) would not trigger this message. (Perhaps we could only turn this on in prerelease (alpha and rc) versions? Alternatively, a change like this could go with the version 5.0 release.)

  • after a while, we switch this to
    Warning: Using old-style Sage command-line options. 

    Sage is changing to use conventional GNU/Posix-style command-line options 
    (for example, 'sage --notebook instead of 'sage -notebook).  This change will 
    become the default soon.  Meanwhile, to use this new style (and therefore 
    to avoid seeing this message), set the environment variable 
    $SAGE_NEW_OPTIONS to something nonempty. 

perhaps with no easy way of disabling this message while using old-style options.

  • finally, we turn on the new options, perhaps with an environment variable $SAGE_OLD_OPTIONS to use the old ones, with the understanding that any changes in command-line options may not be maintained for the old version.

See sage-devel for some discussion.

@jhpalmieri
Copy link
Member

comment:10

I've marked this as "needs review", but it might need work. In the previously cited thread from sage-devel, there was the following suggestion:

Another possibility might be to first check for "--gp", "--gap", etc., 
and do those before doing the general option parsing.   I.e., just do 
what you already planned, but with one optimization to deal with this 
use case. 

This is to speed up access to these programs: do a check like this in a shell script, and then pass the rest of the arguments to Python's optparse using the script included in this patch, or one like it. Then you avoid the slight delay involved in starting up Python if you want to run "gp". It would be nice to have a shell script which had a list of strings "gp", "gap", etc., checked to see if the first(?) argument was "--STR" for STR in this list, and if so, run the appropriate program from SAGE_LOCAL/lib, passing the rest of the line as arguments. Having one list containing all of these strings would make it easy to customize.

@jhpalmieri
Copy link
Member

the file SAGE_ROOT/sage

@jhpalmieri
Copy link
Member

Attachment: sage.gz

Attachment: sage.diff.gz

@jhpalmieri
Copy link
Member

comment:11

Replying to @jhpalmieri:

I've marked this as "needs review", but it might need work. In the previously cited thread from sage-devel, there was the following suggestion:

Another possibility might be to first check for "--gp", "--gap", etc., 
and do those before doing the general option parsing.   I.e., just do 
what you already planned, but with one optimization to deal with this 
use case. 

Okay, here's a new version which does this: it adds a file sage-sage-quickstart which gets run first, implementing the above idea. Then if SAGE_NEW_OPTIONS is nonempty, it calls sage-sage.py, the Python/optparse version with GNU/Posix standard command-line options. Otherwise, it calls the old parser sage-sage.

For the record, the commands in sage-sage-quickstart are: axiom, ecl/lisp, gap, gp, hg, ipython, maxima, mwrank, python, R, singular. Are any others particularly sensitive to startup times? (Using python adds something less than .1 second on my two-year old iMac, so we're not talking about a lot of time, in any case.)

@jhpalmieri
Copy link
Member

comment:12

Note that Python 2.7 will include the argparse module, which might be easier to use than optparse.

@williamstein
Copy link
Contributor Author

comment:13

Wow, this is really fantastic.

@embray
Copy link
Contributor

embray commented Jun 2, 2017

comment:57

Yes, something quite like that.

And I was thinking of writing up some kind of declarative list(s) of subcommands. In particular I was thinking two separate lists:

  1. One list of sage-specific sub-commands (such as sage package in your example above), which would automatically be translated to running individual scripts that implement them that would be named sage-<subcommand>. This is mostly how git works as well.

  2. One list of programs installed in the Sage distribution (sage sh, sage gap, etc.) that can be launched from the interface. In principle one could make this automatic but I think it's better to have a hard-coded list. I don't think a sage run is really necessary. sage sh <whatever> is essentially the same as this, but I think it's still convenient to have shortcuts for common programs included in the Sage distribution.

Although somewhat redundant, because it's common I would also have sage --help as an alias for sage help and sage --help-advanced for sage help --advanced, though one could bikeshed about whether those should do the exact same thing or not.

I might also hide more of the development-specific commands (sage coverage, sage startuptime, etc.) behind a sub-command.

@embray
Copy link
Contributor

embray commented Jun 2, 2017

comment:58

(I should add, that's a very nice mock-up of what such an interface would look like, so thank you for that.)

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2017

comment:59

Replying to @embray:

E.g. replace sage -t with sage test.

What about sage -btp? I use that all the time. I would hate it if that would become
sage buildtest -p or worse, make build && sage test -p.

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2017

comment:60

We should also think to what extent the build system should be exposed under the sage command. For example, we now have

make FOO         # Build dependencies of FOO + FOO
sage -i FOO      # Build toolchain + dependencies of FOO + FOO
sage -f FOO      # Build toolchain + dependencies of FOO + *rebuild* FOO
sage -p FOO      # Build FOO *without* dependencies

and

make sagelib     # Build Sage library with dependencies
sage -i sagelib  # Build Sage library with toolchain and dependencies
sage -b          # Build Sage library *without* dependencies
sage -f sagelib  # Rebuild all of the Sage library with toolchain and dependencies
sage -ba         # Rebuild all of the Sage library *without* dependencies

This is all for historical and accidental reasons, but this ticket should clean that up too.

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2017

comment:61

Needless to say, many people don't even know the subtle differences between the above commands.

@embray
Copy link
Contributor

embray commented Jun 6, 2017

comment:62

Replying to @jdemeyer:

Replying to @embray:

E.g. replace sage -t with sage test.

What about sage -btp? I use that all the time. I would hate it if that would become
sage buildtest -p or worse, make build && sage test -p.

I was actually thinking of allowing subcommands to be chained, like in setup.py. So sage build test, where each can take optional flags if desired.

@jhpalmieri

This comment has been minimized.

@jhpalmieri

This comment has been minimized.

@jhpalmieri

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 11, 2020

comment:66

Strong -1 "wontfix" for the idea of making an incompatible change toward using "subcommands" after 14 years of the existence of the sage script.

@embray
Copy link
Contributor

embray commented Aug 31, 2020

comment:67

I think the current interface of the sage script is pretty clumsy and dated by modern standards. It can and should be made more user-friendly. Just as one example of an advantage of subcommands is it makes the help documentation vastly more digestible. I wouldn't propose changing it without maintaining backwards compatibility though.

@williamstein
Copy link
Contributor Author

comment:68

+1 from me to changing the sage script to support subcommands, especially if we can somehow do it in a way that preserves compatibility with the current parsing. I wonder to what extent the following is possible:

    1. run the current shell script and if it "works" then done (with maybe some slight tightening)
    1. parse using subcommands.

Or something else, e.g,. if you do "sage [explicit list of subcommands]" uses the subcommand approach; otherwise, fall back to the current parser.

I wrote at least the first version of the current sage command line parser, and frankly I didn't know what I was doing at the time, and just sort of stupidly copied random bits and pieces of design from programs I had used. Using python's subcommands support is a lot more systematic, and can also result in very nice modular code.

@embray
Copy link
Contributor

embray commented Sep 1, 2020

comment:69

Since all of the sage script's current "subcommands" (e.g. sage -b, sage -i) all start with hyphens, and new subcommands with be non-hyphenated, I think it would work to support both without too much ambiguity but I'd be interested in a counter-example. sage <filename.{py,sage}> would still work since no sub-command would be confused with a runnable script filename.

@embray
Copy link
Contributor

embray commented Sep 1, 2020

comment:70

Replying to @williamstein:

Using python's subcommands support is a lot more systematic, and can also result in very nice modular code.

I might still just write it as a shell script. Reason being, based on my experience implementing CLIs in Python, it tends to be much much slower to run a single command. At the very least I would do this for the top-level sage script. Most subcommands would delegate to another program which might be another shell script, or could be written in Python (as is already the case). For subcommands written in Python it's not always so bad as long as most operations you would do with that command are long enough to make the Python interpreter startup time negligible. Those could also have further subcommands.

@williamstein
Copy link
Contributor Author

comment:71

I might still just write it as a shell script.

+1

I should have just said "in my experience, structuring command line parsing code as subcommands (implemented in any language) can result in very nice modular code."

@nbruin
Copy link
Contributor

nbruin commented Sep 4, 2020

comment:73

Replying to @williamstein:

+1 from me to changing the sage script to support subcommands,

One very frustrating part of subcommands is the failure of standard tab-completion to work with it:
for instance, for jupyter notebook sheet.ipynb. If there were just a command jupyter-notebook, it would be much easier and faster to type. Particularly with jupyter, which you nearly always use to start its notebook, it's rather frustrating. For git somehow it feels a little more natural, probably because there is naturally a larger variety of actions you want to take through it. It also helps that most preconfigured bash tabcompletions are aware of git subcommands. (although still, distinct commands git-push, git-branch, git-pull would be faster)

@jhpalmieri
Copy link
Member

comment:74

Replying to @nbruin:

Replying to @williamstein:

+1 from me to changing the sage script to support subcommands,

One very frustrating part of subcommands is the failure of standard tab-completion to work with it:

The question so far has been (for example) sage --notebook vs. sage notebook, and tab-completion won't work with either. Are you suggesting adding sage-notebook and other scripts?

@nbruin
Copy link
Contributor

nbruin commented Sep 4, 2020

comment:75

Replying to @jhpalmieri:

The question so far has been (for example) sage --notebook vs. sage notebook, and tab-completion won't work with either. Are you suggesting adding sage-notebook and other scripts?

From a tab-completion point of view that would make sense, yes. (that, or learn how to extend the tab completion patterns). For this particular example, I'd use jupyter notebook anyway, with the sage kernel installed in the system jupyter server.

The traditional reason for having dashes in front of options/subcommands is to remove ambiguity from sage notebook (to run the file notebook) and sage notebook (to start the notebook). For that reason, I think we can only have subcommands for sage if sage <file> would have no meaning. I don't think we can discard this main function of sage. I think with jupyter, where there is one VERY common use, it's already a mistake to go with a subcommand design.

Extrapolating from that, I think that using subcommands for the sage script is also the wrong fit. It works well with git, but users really interact differently with git.

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 4, 2020

comment:76

Replying to @nbruin:

The traditional reason for having dashes in front of options/subcommands is to remove ambiguity from sage notebook (to run the file notebook) and sage notebook (to start the notebook). For that reason, I think we can only have subcommands for sage if sage <file> would have no meaning. I don't think we can discard this main function of sage. I think with jupyter, where there is one VERY common use, it's already a mistake to go with a subcommand design.

Extrapolating from that, I think that using subcommands for the sage script is also the wrong fit. It works well with git, but users really interact differently with git.

I fully agree.

@kini
Copy link
Contributor

kini commented Sep 4, 2020

comment:77

Replying to @nbruin:

(that, or learn how to extend the tab completion patterns).

FWIW, there are tools which can do this for you if your command line subcommands and options are all handled by argparse (rather than e.g. a top level bash script that calls out to python programs that use argparse for each subcommand, as embray suggested).

argcomplete will dynamically provide on-the-fly completion candidates by actually running the argument parsing logic from the sage program every time you hit tab in the shell. This is always accurate but could be slow if sage takes a long time to get to the line of code where the argument parser is run (e.g. if it has some heavy imports).

shtab also runs the argument parsing logic from the sage program, but it statically generates a bash completion script which you can then register with bash-completion by putting it in a relevant place (if you don't have root access, this can be ~/.local/share/bash-completion/completions/). Then when you press tab in the shell, completions should be pretty instantaneous, but the completion script needs to be kept up to date with the command line interface of sage.

If you use a shell other than bash, it may be harder. zsh, at least, is supported by shtab and to some extent argcomplete as well.

I suggest that shtab be run as part of the build process of Sage and that the resulting bash completion script be installed as part of the installation process. That should give out-of-the-box completion functionality to the majority of users, which would be nice. (Again, though, this would only work if the top-level sage does argument parsing in Python with argparse, and I understand that might not end up being the case for other reasons.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants