-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add Git button to Discord RP #111
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.
The config parser is a bit rudimentary but it'll get the job done in case it is needed.
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.
LGTM (though untested because I don't actually use this package).
folder = os.path.dirname(entity) | ||
url = None | ||
try: | ||
url = subprocess.check_output(["git", "-C", folder, "remote", "get-url", "origin"], universal_newlines=True, creationflags=subprocess.CREATE_NO_WINDOW) |
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 flag was added in Python 3.7 and the default plugin host in ST uses 3.3, so it will never work.
You'll need to either use the workaround that is also in place in Default/exec.py
to set the options directly (for Windows only) or tell ST to use the 3.8 host by adding a .python-version
file in the package root with the contents 3.8
, which will also break compatibility with ST <4.
Edit: Actually, ST3 will just try to execute the package with its 3.3 host and run into the exception on this code, causing the fallback config parser to be used, so I guess it would actually be fine to just do that. It obviously needs to be tested whether the package works fine under 3.8 first, though.
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.
Ah whoops, I'll change it to use STARTUPINFO for Windows for the popen options, would that work for v3.3 and versions?
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.
Yeah, that should work for all versions (even ST2 😉).
This adds a button to the rich presence for opening up the current repository URL. It reads the config and get the url from the remote origin, if none is found it will not display the button.
Added two new options to the settings: