-
Notifications
You must be signed in to change notification settings - Fork 87
Conversation
@casperstorm I have the major refactor done that I was talking about and the beginning of the git piece already added. I need to go back through the code and make a lot of comments and audit all my changes to make sure I didn't overlook anything. A basic test seems like everything is working as before. |
5ab6deb
to
f4617ca
Compare
@casperstorm this is in a really good spot with Git support. You can test by installing repos from the command line via The last thing that needs to be built out is a way to add the git repo from the GUI. Have you put anymore though into how you'd like to present this in the GUI? |
I'll test and make GUI design today. |
@casperstorm I connected the install path for git to the gui. The button updates it's text just like catalog, and if an error is encountered, I feed that error text to a new error text container. Once the addon is succesfully installed, I show the install button as "Installed" and disabled. This will also work on already installed git addons, if it shares the URL with what is typed in, it'll recognize it as already installed. |
A few things when you have time, @tarkah:
|
2052cd6
to
b74f320
Compare
@casperstorm I fixed the fingerprint issue... we were using an already escaped string sequence, but putting it in the r"" raw string literal -_____- |
@casperstorm Ok, so now Enter and the button are disabled if already installed / an error is encountered. Once user input is received in the text box, this resets the status. Try it out! |
LGTM. We should soon get it to development I think! |
sounds good! I'll backmerge and wait for @mlablah to do some initial testing, then we can merge |
I'll do some more testing today as well. |
Proposed Changes
Checklist
CHANGELOG.md
if knowledge of this change could be valuable to users