enahce extension manager - #3417
Conversation
- add reveal for token - add update button for custom extensions
![devloai[bot]](https://avatars.githubusercontent.com/in/886113?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary:
- Adds Black formatting cleanup throughout the extension manager script
- Introduces a hold-to-reveal eye button for the token
PasswordBox - Adds an "Update" mode for already-installed custom extensions, allowing users to edit the Git URL and token without reinstalling
Review Summary:
Review covered the three changed files: script.py, ExtensionsWindow.xaml, and the English resource dictionary. Key areas examined: IronPython 2.7 compatibility (UTF-8 header, .NET property access patterns), Black/PEP8 compliance, locale key consistency across all 6 non-English resource dictionaries, token reveal security (mouse event lifecycle), and the update-mode config persistence logic. One security UX issue was found in the reveal button (missing MouseLeave handler), one silent failure that masks a key config write, one redundant import, and a missing locale key in all non-English files.
Suggestions
- Add AddCustomExtension.RevealTokenToolTip key to all 6 non-English locale resource dictionaries (de_de, fr_fr, es_es, ru, pt_br, chinese_s) Apply
- Wire MouseLeave="reveal_token_mouseup" on the reveal button in ExtensionsWindow.xaml to prevent the token staying visible if the user drags the cursor off the button Apply
| pkg.config.token = "" | ||
| except Exception: | ||
| pass | ||
| from pyrevit.userconfig import user_config |
There was a problem hiding this comment.
Redundant import of user_config — it's already imported at module level (line 15: from pyrevit.userconfig import user_config, CONSTS). This local import inside the method shadows the module-level name unnecessarily. Simply remove line 593; user_config.save_changes() on the next line will use the module-level import just fine.
| from pyrevit.userconfig import user_config |
(Delete this line entirely — the module-level user_config import on line 15 is already in scope.)
actions
Feedback: Rate this comment to help me improve future code reviews:
- 👍 Good - Helpful and accurate
- 👎 Poor - Wrong, unclear, or unhelpful
- Skip if you don't have any strong opinions either way.
There was a problem hiding this comment.
yeah, no idea why this reimport is necessary, but it is. "user_config" referenced before assignment otherwise
Great additions, thanks @Wurschdhaud |
- add error debug logging
There was a problem hiding this comment.
Pull request overview
This PR updates the pyRevit Extension Manager UI and logic to improve custom extension credential handling and allow editing settings for already-installed custom extensions.
Changes:
- Reformats
Extensions.smartbuttonlogic (Black-style formatting) and adds a “hold-to-reveal” UI for the Git token field. - Adds an “Update” mode for installed custom extensions, enabling editing of the Git URL/token from the manager UI.
- Adds localized tooltip text for the new token reveal button across supported languages.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py | Adds update-mode behavior, token reveal handlers, and related UI state changes. |
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/ExtensionsWindow.xaml | Reworks token input into PasswordBox + reveal TextBox overlay + reveal button. |
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/ExtensionsWindow.ResourceDictionary.en_us.xaml | Adds localized tooltip for token reveal button. |
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/ExtensionsWindow.ResourceDictionary.de_de.xaml | Adds localized tooltip for token reveal button. |
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/ExtensionsWindow.ResourceDictionary.es_es.xaml | Adds localized tooltip for token reveal button. |
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/ExtensionsWindow.ResourceDictionary.fr_fr.xaml | Adds localized tooltip for token reveal button. |
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/ExtensionsWindow.ResourceDictionary.pt_br.xaml | Adds localized tooltip for token reveal button. |
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/ExtensionsWindow.ResourceDictionary.ru.xaml | Adds localized tooltip for token reveal button. |
| extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/ExtensionsWindow.ResourceDictionary.chinese_s.xaml | Adds localized tooltip for token reveal button. |
| # Write updated values to the extension config | ||
| pkg.url = new_url | ||
| try: | ||
| pkg.config.url = new_url | ||
| except Exception as e: |
There was a problem hiding this comment.
Added alert message for now - keeping as TODO. Possibly restart is the only option in case the URL changed, got to investigate.
…abel, remove stale reimport - Add _clear_token_ui() helper; call it in _update_add_custom_section_for_new and in the builtin/not-installed branches of _update_add_custom_section_for_selection to prevent a previously-loaded token leaking into an unrelated flow - Guard git_info_text_changed label reset behind `not self._update_mode` so the "Update" button label survives typing in the URL field - Remove redundant `from pyrevit.userconfig import user_config` re-import and its TODO comment from the update path; module-level import is sufficient - Clarify update-path alert: config/token are saved but the installed repo's git remote is not retargeted; instruct user to run `git remote set-url` - Remove comment about 5.x behaviour (irrelevant for current code)
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26161+1943-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+0730-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+0850-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New work-in-progress (wip) builds are available for 6.4.0.26166+1546-wip |
|
📦 New public release is available for 6.5.0.26173+1406 |
1 similar comment
|
📦 New public release is available for 6.5.0.26173+1406 |
Description
Checklist
Before submitting your pull request, ensure the following requirements are met:
pipenv run black {source_file_or_directory}Additional Notes
related PR #3296 will need to be updated as well, will for sure have merge conflicts