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

Restore --grep and --grepdoc command line options #12333

Closed
williamstein opened this issue Jan 21, 2012 · 13 comments
Closed

Restore --grep and --grepdoc command line options #12333

williamstein opened this issue Jan 21, 2012 · 13 comments
Assignees
Milestone

Comments

@williamstein
Copy link
Contributor

I just tried one of my favorite command line options in Sage-4.8:

$ sage -grep optional |grep gap # and all kinds of other awesome command line stuff 

and was very surprised and disappointed that (a) sage -grep is just gone, and (b) there is no other equivalent command line functionality.

Searching reveals that #12190 removed sage -grep from sage. You can't just remove functionality without any comment. At a minimum it would have to be deprecated for a year, and should also be discussed on sage-devel.

I'm able to accomplish what I want via

./sage -c "search_src('optional', 'gap')"

However, this requires starting up Sage, which is much slower than the old sage -grep. I admit that "-grep" wasn't the best command line option name -- search_src would have been better.

The optimal fix for this ticket would be to refactor the code in sage/misc/sagedoc.py (if possible) so it can be imported without importing all Sage, then use it to implement a command line "-search_src", and add back "-grep" with a 1-year deprecation.

Failing that, just revert #12190 for 1 year.


Apply:

Component: misc

Author: Francis Clarke

Reviewer: William Stein, John Palmieri

Merged: sage-5.0.beta6

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

@sagetrac-fwclarke
Copy link
Mannequin

sagetrac-fwclarke mannequin commented Jan 25, 2012

Author: Francis Clarke

@sagetrac-fwclarke
Copy link
Mannequin

sagetrac-fwclarke mannequin commented Jan 25, 2012

comment:1

Attachment: trac_12333_restore_sage_-grep.patch.gz

The patch restore sage -grep, and sage -grepdoc, but uses a new simpler, one-line grep. This has the following advantages:

  • It is significantly faster;
  • The files are searched in lexicographical order, rather than putting all the ".py" files before the ".pyx" files, etc.

In addition a new header is provided, which doesn't include the irrelevant "Type notebook() ...", and which shows the branch (if it isn't main).

@jhpalmieri
Copy link
Member

comment:2

Unfortunately, grep --recursive ... does not work on Solaris or OpenSolaris. These are supported platforms for Sage, so we need another approach.

As far as the banner occurs, maybe it shouldn't be there at all? Set SAGE_BANNER=no, as in sage -c ..., to omit it.

Finally, this should be rebased to some beta release of Ssge 5.0: the script sage-sage is no longer there; instead, patch SAGE_ROOT/spkg/bin/sage.

@williamstein
Copy link
Contributor Author

comment:3

Replying to @jhpalmieri:

Unfortunately, grep --recursive ... does not work on Solaris or OpenSolaris. These are supported platforms for Sage, so we need another approach.

But please keep grep --recursive for everything but Solaris.

As far as the banner occurs, maybe it shouldn't be there at all? Set SAGE_BANNER=no, as in sage -c ..., to omit it.

Agreed.

Finally, this should be rebased to some beta release of Ssge 5.0: the script sage-sage is no longer there; instead, patch SAGE_ROOT/spkg/bin/sage.

@sagetrac-fwclarke
Copy link
Mannequin

sagetrac-fwclarke mannequin commented Jan 27, 2012

Attachment: trac_12333_restore_grep_root.patch.gz

apply to root repository

@sagetrac-fwclarke
Copy link
Mannequin

sagetrac-fwclarke mannequin commented Jan 27, 2012

apply to scripts repository

@sagetrac-fwclarke
Copy link
Mannequin

sagetrac-fwclarke mannequin commented Jan 27, 2012

comment:4

Attachment: trac_12333_restore_grep_scripts.patch.gz

Replying to @jhpalmieri:

Unfortunately, grep --recursive ... does not work on Solaris or OpenSolaris.

The code in the new attachment: trac_12333_restore_grep_scripts.patch should work much more generally. It seems to be no slower than the previous approach.

As far as the banner occurs, maybe it shouldn't be there at all? Set SAGE_BANNER=no, as in sage -c ..., to omit it. Finally, this should be rebased to some beta release of Sage 5.0: the script sage-sage is no longer there; instead, patch SAGE_ROOT/spkg/bin/sage.

The attachment attachment: trac_12333_restore_grep_root.patch patches SAGE_ROOT/spkg/bin/sage, removes the banner, and allows -search_src as a synonym for -grep, and -search_doc for -grepdoc.

One other modification to earlier behaviour: the grep option -i is no longer hard-wired, but it can be restored, as can other options. For example:

$ sage -search_src -il clarke
sage/rings/morphism.pyx
sage/rings/number_field/number_field.py
sage/rings/number_field/number_field_element.pyx
sage/rings/number_field/number_field_ideal.py
sage/rings/number_field/unit_group.py
sage/rings/polynomial/polynomial_element_generic.py

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:5

Looks good, works on sage.math, OS X, OpenSolaris. So there don't seem to be any cross-platform issues.

I'm happy with the two main patches: positive review for those. What do you think of the attached patch for the reference manual?

@jhpalmieri
Copy link
Member

Reviewer: William Stein, John Palmieri

@jhpalmieri
Copy link
Member

Sage library

@sagetrac-fwclarke
Copy link
Mannequin

sagetrac-fwclarke mannequin commented Feb 26, 2012

comment:6

Attachment: trac_12333-documentation.patch.gz

Replying to @jhpalmieri:

... What do you think of the attached patch for the reference manual?

Looks good to me, so positive review for the documentation change.

@jdemeyer jdemeyer changed the title Add back functionality that was inadvertantly deleted from Sage Restore --grep and --grepdoc command line options Feb 26, 2012
@jdemeyer
Copy link

Merged: sage-5.0.beta6

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

4 participants