Skip to content

Conversation

@xavier-ateo
Copy link

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

  • I took over the previously existing logic for determining OneDrive user folders. However I did not test if it still works as intended.
  • I also did not test it under Windows 10.


# Configure logging
if DEBUG_LOGGING:
logging.basicConfig(level=logging.DEBUG, format="%(levelname)s: %(message)s")
Copy link
Collaborator

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",
Copy link
Collaborator

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

Copy link
Author

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
Copy link
Collaborator

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.

Comment on lines 46 to 47
if DEBUG_LOGGING:
logging.basicConfig(level=logging.DEBUG, format="%(levelname)s: %(message)s")
Copy link
Collaborator

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__)
. Then generally you can just set the log level based on the Talon setting as above and the overhead from constructing log messages will be negligible when compared with the verbosity repeated checks add to your code.

Copy link
Author

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"]
Copy link
Collaborator

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?

Copy link
Author

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."""
Copy link
Collaborator

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.

Copy link
Author

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?

Copy link
Collaborator

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.

Copy link
Collaborator

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.

nriley and others added 4 commits September 13, 2025 20:36
… 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
@xavier-ateo
Copy link
Author

I have reworked the code quite a bit:

  • It now uses Windows APIs for resolving the user folder display names and paths. This removes the need for the previous OneDrive logic.
  • It now uses a local logger instance which can be enabled when adding a new langauge mapping
  • Removed docstrings from UserActions methods
  • Lots of code restructuring

What is still an issue:

  • Nested localized standard folders (e.g., "Pictures/Screenshots") don't work with the file manager commands
  • WSL root folders (e.g., "Linux/Ubuntu") don't work with the file manager commands

Languages still need to be implemented manually because:

  • Windows 11 returns a localized window title if multiple tabs are open ("Documents and 1 more tab - File Explorer"). I could not find a way to solve this via an API.
  • Window titles such as "Gallery", "Home", "Network" I haven't yet figured out. Will look into it though.

What I also did:

  • Removed the following excluded window titles, because I could not see when the Explorer would ever show those as titles:
    "Task Switching", "Task View", "Program Manager", "Run"

Copy link
Collaborator

@nriley nriley left a 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?

Comment on lines +163 to +165
guid.Data1 = int(guid_string[0:8], 16)
guid.Data2 = int(guid_string[8:12], 16)
guid.Data3 = int(guid_string[12:16], 16)
Copy link
Collaborator

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!

@xavier-ateo
Copy link
Author

No worries about the delay! I don't plan to make any additional changes. You can merge it anytime you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants