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

There are more instances of GitGutterSettings than one #334

Closed
rchl opened this issue Nov 18, 2016 · 3 comments
Closed

There are more instances of GitGutterSettings than one #334

rchl opened this issue Nov 18, 2016 · 3 comments
Labels

Comments

@rchl
Copy link
Collaborator

rchl commented Nov 18, 2016

This:

    from git_gutter_settings import settings

doesn't work like I expected. It evaluates the git_gutter_settings.py file in the context of the importee and thus we end up using different instances of GitGutterSettings in different parts of the code. That is generally not a big problem apart when using flags like self._git_binary_path_error_shown where we rely on there being only one instance of GitGutterSettings.

I'm not sure what's the typical way of creating singletons in Python but that doesn't seem to be the right way.

@r-stein
Copy link
Collaborator

r-stein commented Nov 18, 2016

As far I know there is one set of moduls, which is initially loaded and an other set, which is used if you reload a module (e.g. work on it and save it or update it).
This could be avoided by using from GitGutter.git_gutter_settings import settings, but this would be problematic with the edge version.
In addition we could try in the git_gutter_settings.py something like (I haven't tested it):

if "settings" not in globals():
    settings = ...

@rchl
Copy link
Collaborator Author

rchl commented Nov 18, 2016

IMO we don't really need to handle case when GitGutter code is modified. That's an edge case that should only affect GitGutter developers.
It would be enough that various git_gutter_*.py files use the same instance which is currently not the case.

@rchl
Copy link
Collaborator Author

rchl commented Nov 18, 2016

Your suggestion using global() seems to work. And the global doesn't leak to other packages so it appears to be safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants