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

Option to limit search result in history magic command #2404

Merged
merged 8 commits into from Nov 3, 2012

Conversation

tkf
Copy link
Contributor

@tkf tkf commented Sep 12, 2012

To use the new limit option introduced in #2393.

Diff is rather big for what it really does because I need argparse and custom action to parse argument without changing other semantics.

BTW probably 3e6f57d (magic_arguments.py refactoring) is not needed. I just couldn't stand introducing duplicated code. But removing it should not change the functionality.

@@ -16,10 +16,13 @@
# Stdlib
import os
from io import open as io_open
from argparse import Action
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you import it from IPython.external ?
otherwise it fails on python 2.6

@Carreau
Copy link
Member

Carreau commented Sep 13, 2012

I didn't fully try, but looks ok to me once the small small comment I made is fixed.
Thanks !

@bfroehle
Copy link
Contributor

What is the new feature here? i.e., what %history command can I do now that I couldn't before?

@@ -27,16 +30,75 @@
# Magics class implementation
#-----------------------------------------------------------------------------


class HistoryArgLimitAction(Action):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A custom action is probably overkill. It's probably easier to just do

_unspecified = object()

# ...
@magic_arguments.argument(
    '-l', dest='limit', type=int, nargs='?', default=_unspecified,
    ...
)
# ...
def history(self, line):
#...

    if args.limit is None:
        args.limit = 10
    elif args.limit is _unspecified:
        args.limit = None

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree with @bfroehle on this one.

@tkf
Copy link
Contributor Author

tkf commented Sep 14, 2012

@bfroehle You can limit history search result using -l flag once this is merged.

%history -l -g SEARCH WORD        # same as -l 10
%history -l 5 -g SEARCH WORD

@Carreau
Copy link
Member

Carreau commented Sep 15, 2012

This does not pass the test on python 2.6 because of argparse that should be imported from IPython external where we do the check of wether system argparse exist and load the one we ship if it does not.

@tkf
Copy link
Contributor Author

tkf commented Sep 15, 2012

@Carreau Thanks, I noticed your first comment. I just don't have time to fix it. I should have told you that I acknowledged it.

@Carreau
Copy link
Member

Carreau commented Sep 15, 2012

No problem, I just now that when discussion start on another topic, at least I, tend to forgot the things I have to fix in previous messages.

@tkf
Copy link
Contributor Author

tkf commented Sep 16, 2012

Fixed

@tkf
Copy link
Contributor Author

tkf commented Oct 5, 2012

Applied the suggestion by @bfroehle to not use the custom action.

@Carreau
Copy link
Member

Carreau commented Oct 31, 2012

Hi,

I re-read quickly and didn't saw anything wrong.
I'll leave it a day or two and merge.

seen as 'get_ipython().magic("%%cd /")' instead of '%%cd /'.
""")
@argument(
'-f', dest='filename',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this missing nargs=1 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nargs=1 is default behavior for optional argument

In [4]:
import argparse
parser = argparse.ArgumentParser()
parser.add_argument('--filename')
parser.add_argument('positional')
vars(parser.parse_args(['--filename', 'a', 'b']))
Out [4]:
{'filename': 'a', 'positional': 'b'}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it works that way:

% ./ipython.py -c '%history -f out.log -g import' && head out.log
 9/2: import pylab
 9/3: import numpy
10/1: import numpy
12/1: import numpy
13/1: import pylab
14/1: import pylab
15/1: import numpy
17/11: ';'.join(__IP.complete('''import'''))
17/12: import numpy
17/13: import sys

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, my bad. :/ You are correct.

@bfroehle
Copy link
Contributor

I think there might be a little bug with -f but otherwise okay.

Carreau added a commit that referenced this pull request Nov 3, 2012
Option to limit search result in history magic command
@Carreau Carreau merged commit fb4a45e into ipython:master Nov 3, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Option to limit search result in history magic command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants