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

Fix history magic command's bugs wrt to full history and add -O option to display full history #235

Closed
wants to merge 3 commits into from

Conversation

madhusudancs
Copy link

Hi Fernando, These are my patches for the history magic command's bugs we came across during SciPy.in 2010. I have also added -O option. I have tried my best to make sure that the input_hist_raw/_parsed variables are changed in all the relevant places where they must be changed. But more eyes, the merrier! Just drawing your attention towards it. As you had suggested, I will be adding tests to the changes I have made in the next few days. Sending this pull request before that, so that the patches get reviewed as I spend time writing tests.

Also, would like to add credits to two guys who helped me show one of the bugs with -o option. Their names are '"Vinay Srinivas" srinivasvinay.246@gmail.com' and '"Vishnu SG" sgvishnu777@gmail.com'. I forgot to add their names in the commit. Is it possible to add them elsewhere?

…t history lists.

Currently by assigning the history data read from the JSON file to
a new history list object we are losing the pointer to the existing
lists. So instead we should clear the existing list objects and
assign the read data to it.
…ir names.

Some history can be shown only for the inputs entered during the current session.
For example -n option to display the line numbers doesn't have any meaning for
history from previous sessions since they can be from multiple sessions which
means we may end up with say, multiple line numbers like 1, 2, 3, etc. for each
session. So we separate the list that holds the history into two lists one set
holding the history of the current session and the other set holding the full
history.

For history magic command and all its options we only display the history from
the current session.
…ry too.

All the options for which the meaningful history cannot be displayed for
the history from previous sessions are overridden by this option. This is
documented in the docstring too. Example of -n option is given in the
previous commit.
@fperez
Copy link
Member

fperez commented Feb 22, 2011

Hey Madhu,

many thanks for this! I'm really sorry that in a lot of travel I've had since India, I completely fell behind on IPython and never worked on this. Totally my fault, and for that I apologize.

You may have seen on the list that Thomas has actually tackled a major refactor of the history system that moves to an SQLite backend, so it may be that much of what Satra, you, others and I did in India will get significantly redone with this approach. So let's give this a few more days until we see if Thomas' ideas pan out.

I encourage you to participate in the discussions on the list if you have any thoughts on that stuff. And sorry again for falling behind on the code review!

@madhusudancs
Copy link
Author

Hey Fernando,
No problem at all. No need to say sorry at all. I have fallen behind on the stuff too. Although I have been seeing the discussion going on the history stuff, I have not been able to mails in detail because of the travel and stuff. Sure, I will read them and jump in whenever I can.

@fperez
Copy link
Member

fperez commented Feb 22, 2011

OK, I'm very glad to hear that. The project is certainly picking up momentum, and Thomas has been doing a great job ensuring that a lot of the work done in India isn't lost despite my limitations (even if in this case, it seems there was a bit of unnecessary duplication, unfortunately).

@takluyver
Copy link
Member

Thanks for this, Madhu - I think I've got all the magic commands sorted in the move to SQLite history, which I've just merged, but please do grab trunk and test them out.

@takluyver takluyver closed this Mar 24, 2011
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