-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
Expose settings API to other handlers. #94
Conversation
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.
Cheers @goanpeca!
This is a good solution to how server side extensions access user settings.
Ok, let me add some tests then :-p |
Is it possible to combine |
Yeah I think so. Will update the code. |
11b062f
to
be605f3
Compare
@afshin I did something else, Since now the get method is using the same thing as the API (public method) I think the current tests are covering the use cases. |
a7a4c05
to
393694b
Compare
path = _path(settings_dir, schema_name, False, SETTINGS_EXTENSION) | ||
raw = '{}' | ||
settings = dict() | ||
settings = {} |
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.
This is faster to use than dict
, so .. why not use it 🙃
4ee5ef2
to
d99ea9f
Compare
d99ea9f
to
d3a035a
Compare
Published as 1.2.0, JupyterLab will need an update: https://github.com/jupyterlab/jupyterlab/blob/master/setup.py#L154 |
Hi @afshin, would something like this be ok so other handlers can access the settings in a friendlier fashion?
On the localization work, I use the current locale set by the user and based on that I provide the available language in original and localized form (for the current local)
Thoughts?