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
Split standard input line history per session (notebook) #13944
Split standard input line history per session (notebook) #13944
Conversation
Thanks for making a pull request to jupyterlab! |
fb759ca
to
fcf08b6
Compare
The trick to getting this to work turned out to be buried in the kernel msg that's used to request the So technically this ended up being a "split stdin history by kernel session" feature rather than being "split by notebook", but I actually think it's better this way. @krassowski @fcollonval Could one of you please take a look at this when you have a sec? Ideally I'd like to get this in before the feature freeze on Wednesday (though I realize that may be a heavy ask). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @telamonian
I tested it on Binder. It works as expected and the history is kept shared for console for example.
On the implementation level, we should move the configuration flag in INotebookConfig
as this is not related to the editor
The current code will also not be possible if you rebase on master - so I advice you to first move to notebook config before rebasing.
This means also you need to move the settings as a primary property like maxNumberOutputs
.
Regarding the history storage, I would prefer using a Map
rather than a object. And we should try to clean the history if the session gets disposed. But that seems quite complex for this small amount of data; something like a static public method on Stdin
triggered by session.disposed
?
This could be a |
So doing this with a static method sounds nice (though introduces an additional dependency on Using a Given the tiny benefit of cleaning it up I propose we move forward without it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @krassowski for finishing up this one
Benchmark reportThe execution time (in milliseconds) are grouped by test file, test type and browser. The mean relative comparison is computed with 95% confidence. Results table
Changes are computed with expected as reference.
Waiting for localhost:8888 Cell memory leaksCreate a code cellMemory change: -144 kB Leak detected: NoLeaking objects:
Leaking collections: Create a markdown cellMemory change: -149 kB Leak detected: NoLeaking objects:
Leaking collections: Create a raw cellMemory change: +149 kB Leak detected: YesLeaking objects:
Leaking collections:
File editor memory leaksCreate a fileMemory change: -79.4 kB Leak detected: NoLeaking objects:
Notebook memory leaksCreate a notebookMemory change: +27.5 kB Leak detected: YesLeaking objects:
2 passing (7m)
|
References
Closes #13798
Code changes
inputHistoryScope
setting to notebook (and intialization options of:Cell
,OutputArea
andStdin
)User-facing changes
Input History Scope
settingBackwards-incompatible changes
NA