-
Notifications
You must be signed in to change notification settings - Fork 838
Windows Explorer Integration: Added support for other languages & tabs (Win 11) + code refactoring #2020
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
base: main
Are you sure you want to change the base?
Conversation
…orer tabs in Windows 11 + code refactoring
for more information, see https://pre-commit.ci
|
|
||
| # Configure logging | ||
| if DEBUG_LOGGING: | ||
| logging.basicConfig(level=logging.DEBUG, format="%(levelname)s: %(message)s") |
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.
remove this please, talon already enables logging
| ], | ||
| "folder_names": { # Right side is the localized name | ||
| "Desktop": "Desktop", | ||
| "Documents": "Dokumente", |
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.
surely there's a win32 api to get the names for these folders rather than hardcoding per language
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.
You are right, there is a win32 API. I will use that instead of the hardcoded values for the user folder names.
| from talon import Context, Module, actions, app, ui | ||
|
|
||
| # Configuration | ||
| DEBUG_LOGGING = False |
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.
In general, we tend to use Talon settings rather than constants in the code to configure logging verbosity.
| if DEBUG_LOGGING: | ||
| logging.basicConfig(level=logging.DEBUG, format="%(levelname)s: %(message)s") |
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.
If you want to declare a logger that is private to your module you can use something like
| logger = logging.getLogger(__name__) |
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.
I'll do that, thanks
| current_language_mapping["folder_names"], get_folder_path, user_display_name | ||
| ) | ||
|
|
||
| directories_to_exclude = [""] + current_language_mapping["excludes"] |
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.
Can you explain why you have an empty string in this list?
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.
It was part of the original implementation, therefore I kept it. I don't know if it is needed or not.
directories_to_exclude = [
"",
"Run",
"Task Switching",
"Task View",
"This PC",
"File Explorer",
"Program Manager",
| class UserActions: | ||
|
|
||
| def file_manager_open_parent(): | ||
| """Navigate to parent directory.""" |
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.
In general, we do not put docstrings in a context implementation, just in the module definition of an action.
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 original implementation also had docstrings. Should I remove all of them?
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.
Please, as long as there is nothing missing from the module docstring.
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.
Please, as long as there is nothing missing from the module docstring.
… API integration Refactoring of previous Windows Explorer integration: **Improvements:** - Use Windows API for user folder display names instead of hardcoded values - Use Windows API for user folder paths. This removes the need for the previous OneDrive user folder detection - Simplified language mapping structure and removed merging logic - Reorganized code into logical sections **Code review adjustments:** - Use "logger = logging.getLogger(__name__)" for logging - Removed docstrings from UserActions methods
for more information, see https://pre-commit.ci
|
I have reworked the code quite a bit:
What is still an issue:
Languages still need to be implemented manually because:
What I also did:
|
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.
From the community backlog session — apologies for the delay in review, as we did not realize that you had made some additional changes. This is looking really good! Do you want to have us consider merging it soon and make the other changes in a future PR?
| guid.Data1 = int(guid_string[0:8], 16) | ||
| guid.Data2 = int(guid_string[8:12], 16) | ||
| guid.Data3 = int(guid_string[12:16], 16) |
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 Python uuid.UUID object can give you these with .time_low, .time_mid and .time_hi_version. I don't see a way to get 8 individual bytes so you'll have to do that yourself!
|
No worries about the delay! I don't plan to make any additional changes. You can merge it anytime you want. |
The file picker GUI for Explorer did not work for me on Windows 11 24H2. I have addressed several issues:
Multiple language support
Previously, the Windows Explorer integration was only targeted towards English as the operating system language. It now also supports German, and other languages can be easily added.
Windows 11 tabs
Path detection was broken when multiple tabs were open in one Explorer window. This is now fixed for English and German.
Logging and code refactoring
Logging can be easily enabled to debug issues and figure out which format a specific language uses when you want to add support for it.
Todo