-
-
Notifications
You must be signed in to change notification settings - Fork 457
Enhancement: new builtin shortcuts: active_office_file #3046
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: dev
Are you sure you want to change the base?
Conversation
Fixes Flow-Launcher#2936 Add new builtin shortcut `{active_office_file}` to quickly access the active office file. * Add a new method `GetActiveOfficeFilePath` in `Flow.Launcher.Infrastructure/Helper.cs` to retrieve the active office file path. * Add a new builtin shortcut `{active_office_file}` in `Flow.Launcher.Infrastructure/UserSettings/Settings.cs` to use the `GetActiveOfficeFilePath` method. * Update the `BuiltinShortcuts` list in `Flow.Launcher.Infrastructure/UserSettings/Settings.cs` to include the new `{active_office_file}` shortcut. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/Flow-Launcher/Flow.Launcher/issues/2936?shareId=XXXX-XXXX-XXXX-XXXX).
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Using If the flagged items are 🤯 false positivesIf items relate to a ...
|
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.
Pull Request Overview
This PR adds a new builtin shortcut {active_office_file} to retrieve the file path of the currently active Microsoft Office document (Word, Excel, or PowerPoint). However, the implementation contains several critical bugs that prevent it from compiling or functioning correctly.
Key Changes
- Added
GetActiveOfficeFilePath()method inHelper.csto retrieve the active Office file path - Registered the new
{active_office_file}shortcut inSettings.csBuiltinShortcuts collection
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 14 comments.
| File | Description |
|---|---|
| Flow.Launcher.Infrastructure/Helper.cs | Adds P/Invoke declarations and COM automation logic to detect and retrieve the active Office file path from Word, Excel, or PowerPoint applications |
| Flow.Launcher.Infrastructure/UserSettings/Settings.cs | Registers the new {active_office_file} builtin shortcut with localization key and handler method |
Critical Issues Found:
The implementation has fundamental bugs that will prevent compilation:
- Incorrect namespace usage -
Microsoft.Win32is used as an alias but doesn't contain the required P/Invoke methods (OpenProcess,GetModuleFileNameEx) or theDispatchtype for COM interop - Missing proper P/Invoke declarations for Win32 APIs
- Missing error handling for COM exceptions and edge cases
- Resource leak - process handles are not closed
- Nullable return type not declared despite returning null
The code needs significant rework using proper P/Invoke declarations and COM interop patterns (similar to FileExplorerHelper.cs) before it can function correctly.
Comments suppressed due to low confidence (1)
Flow.Launcher.Infrastructure/Helper.cs:121
- This assignment to threadProcessId is useless, since its value is never read.
var threadProcessId = GetWindowThreadProcessId(window, out var processId);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| else | ||
| { | ||
| return null; |
Copilot
AI
Nov 18, 2025
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 method has #nullable enable at the top of the file but returns null on line 114 without declaring the return type as nullable. The method signature should be:
public static string? GetActiveOfficeFilePath()This matches the nullable pattern used by GetActiveExplorerPath() in FileExplorerHelper.cs.
| using System.Diagnostics; | ||
| using System.Runtime.InteropServices; | ||
| using System.Linq; | ||
| using System.Collections.Generic; |
Copilot
AI
Nov 18, 2025
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.
Several of these using statements appear to be unused based on the actual code:
System.Diagnosticsis not usedSystem.Linqis not usedSystem.Collections.Genericis not used
Remove unused imports to keep the code clean.
| using System.Diagnostics; | |
| using System.Runtime.InteropServices; | |
| using System.Linq; | |
| using System.Collections.Generic; | |
| using System.Runtime.InteropServices; |
| var handle = win32api.OpenProcess(win32api.PROCESS_QUERY_INFORMATION | win32api.PROCESS_VM_READ, false, pid); | ||
| var exePath = win32api.GetModuleFileNameEx(handle, 0); |
Copilot
AI
Nov 18, 2025
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.
These lines attempt to use non-existent methods from Microsoft.Win32. The OpenProcess, PROCESS_QUERY_INFORMATION, PROCESS_VM_READ, and GetModuleFileNameEx do not exist in this namespace.
You need to:
- Add proper P/Invoke declarations for
OpenProcessandGetModuleFileNameExfrom kernel32.dll - Define the
PROCESS_QUERY_INFORMATIONandPROCESS_VM_READconstants - Properly handle and close the process handle to avoid resource leaks
Example:
[DllImport("kernel32.dll", SetLastError = true)]
private static extern IntPtr OpenProcess(uint processAccess, bool bInheritHandle, int processId);
[DllImport("kernel32.dll", SetLastError = true)]
private static extern bool CloseHandle(IntPtr hObject);
private const uint PROCESS_QUERY_INFORMATION = 0x0400;
private const uint PROCESS_VM_READ = 0x0010;| public static string GetActiveOfficeFilePath() | ||
| { | ||
| var pid = GetActiveWindowProcessId(); | ||
| var handle = win32api.OpenProcess(win32api.PROCESS_QUERY_INFORMATION | win32api.PROCESS_VM_READ, false, pid); |
Copilot
AI
Nov 18, 2025
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 process handle obtained from OpenProcess is never closed, causing a resource leak. Process handles should be closed when no longer needed using CloseHandle.
Consider using a using statement with a SafeHandle wrapper, or ensure the handle is closed in a finally block:
IntPtr handle = IntPtr.Zero;
try
{
handle = OpenProcess(...);
// use handle
}
finally
{
if (handle != IntPtr.Zero)
CloseHandle(handle);
}| if (exePath.ToLower().Contains("winword.exe")) | ||
| { | ||
| return Path.GetFullPath(new win32api.Dispatch("Word.Application").ActiveDocument.FullName); | ||
| } | ||
| else if (exePath.ToLower().Contains("powerpnt.exe")) | ||
| { | ||
| return Path.GetFullPath(new win32api.Dispatch("PowerPoint.Application").ActivePresentation.FullName); | ||
| } | ||
| else if (exePath.ToLower().Contains("excel.exe")) |
Copilot
AI
Nov 18, 2025
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.
Using ToLower() with culture-sensitive comparison for file path matching can cause issues in certain locales. Use StringComparison.OrdinalIgnoreCase instead:
if (exePath.Contains("winword.exe", StringComparison.OrdinalIgnoreCase))Apply the same change to lines 104 and 108 for PowerPoint and Excel checks.
| if (exePath.ToLower().Contains("winword.exe")) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("Word.Application").ActiveDocument.FullName); | |
| } | |
| else if (exePath.ToLower().Contains("powerpnt.exe")) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("PowerPoint.Application").ActivePresentation.FullName); | |
| } | |
| else if (exePath.ToLower().Contains("excel.exe")) | |
| if (exePath.Contains("winword.exe", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("Word.Application").ActiveDocument.FullName); | |
| } | |
| else if (exePath.Contains("powerpnt.exe", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("PowerPoint.Application").ActivePresentation.FullName); | |
| } | |
| else if (exePath.Contains("excel.exe", StringComparison.OrdinalIgnoreCase)) |
| var pid = GetActiveWindowProcessId(); | ||
| var handle = win32api.OpenProcess(win32api.PROCESS_QUERY_INFORMATION | win32api.PROCESS_VM_READ, false, pid); | ||
| var exePath = win32api.GetModuleFileNameEx(handle, 0); | ||
| if (exePath.ToLower().Contains("winword.exe")) | ||
| { | ||
| return Path.GetFullPath(new win32api.Dispatch("Word.Application").ActiveDocument.FullName); | ||
| } | ||
| else if (exePath.ToLower().Contains("powerpnt.exe")) | ||
| { | ||
| return Path.GetFullPath(new win32api.Dispatch("PowerPoint.Application").ActivePresentation.FullName); | ||
| } | ||
| else if (exePath.ToLower().Contains("excel.exe")) | ||
| { | ||
| return Path.GetFullPath(new win32api.Dispatch("Excel.Application").ActiveWorkbook.FullName); | ||
| } | ||
| else | ||
| { |
Copilot
AI
Nov 18, 2025
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 method lacks error handling for several failure scenarios:
- No active window (foreground window could be null)
- Process access denied
- Invalid process ID
- Office application not running or COM exceptions
- No active document/workbook/presentation
The method should wrap these operations in try-catch blocks and return null on any error, matching the pattern of GetActiveExplorerPath() which safely returns null when no explorer is found.
Consider adding comprehensive error handling:
public static string GetActiveOfficeFilePath()
{
try
{
// existing logic
}
catch (Exception)
{
return null;
}
}| var pid = GetActiveWindowProcessId(); | |
| var handle = win32api.OpenProcess(win32api.PROCESS_QUERY_INFORMATION | win32api.PROCESS_VM_READ, false, pid); | |
| var exePath = win32api.GetModuleFileNameEx(handle, 0); | |
| if (exePath.ToLower().Contains("winword.exe")) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("Word.Application").ActiveDocument.FullName); | |
| } | |
| else if (exePath.ToLower().Contains("powerpnt.exe")) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("PowerPoint.Application").ActivePresentation.FullName); | |
| } | |
| else if (exePath.ToLower().Contains("excel.exe")) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("Excel.Application").ActiveWorkbook.FullName); | |
| } | |
| else | |
| { | |
| try | |
| { | |
| var pid = GetActiveWindowProcessId(); | |
| var handle = win32api.OpenProcess(win32api.PROCESS_QUERY_INFORMATION | win32api.PROCESS_VM_READ, false, pid); | |
| var exePath = win32api.GetModuleFileNameEx(handle, 0); | |
| if (exePath.ToLower().Contains("winword.exe")) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("Word.Application").ActiveDocument.FullName); | |
| } | |
| else if (exePath.ToLower().Contains("powerpnt.exe")) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("PowerPoint.Application").ActivePresentation.FullName); | |
| } | |
| else if (exePath.ToLower().Contains("excel.exe")) | |
| { | |
| return Path.GetFullPath(new win32api.Dispatch("Excel.Application").ActiveWorkbook.FullName); | |
| } | |
| else | |
| { | |
| return null; | |
| } | |
| } | |
| catch (Exception) | |
| { |
|
|
||
| private static int GetActiveWindowProcessId() | ||
| { | ||
| var window = GetForegroundWindow(); |
Copilot
AI
Nov 18, 2025
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.
Replace this call with a call to managed code if possible.
| private static int GetActiveWindowProcessId() | ||
| { | ||
| var window = GetForegroundWindow(); | ||
| var threadProcessId = GetWindowThreadProcessId(window, out var processId); |
Copilot
AI
Nov 18, 2025
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.
Replace this call with a call to managed code if possible.
| var window = GetForegroundWindow(); | ||
| var threadProcessId = GetWindowThreadProcessId(window, out var processId); | ||
| return processId; | ||
| } | ||
|
|
||
| [DllImport("user32.dll")] | ||
| private static extern IntPtr GetForegroundWindow(); | ||
|
|
||
| [DllImport("user32.dll", SetLastError = true)] | ||
| private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out int lpdwProcessId); | ||
| } |
Copilot
AI
Nov 18, 2025
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.
Minimise the use of unmanaged code.
| var window = GetForegroundWindow(); | |
| var threadProcessId = GetWindowThreadProcessId(window, out var processId); | |
| return processId; | |
| } | |
| [DllImport("user32.dll")] | |
| private static extern IntPtr GetForegroundWindow(); | |
| [DllImport("user32.dll", SetLastError = true)] | |
| private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out int lpdwProcessId); | |
| } | |
| // Using unmanaged code via NativeMethods to get the foreground window process ID. | |
| var window = NativeMethods.GetForegroundWindow(); | |
| var threadProcessId = NativeMethods.GetWindowThreadProcessId(window, out var processId); | |
| return processId; | |
| } | |
| /// <summary> | |
| /// Encapsulates unmanaged code required for interacting with Windows API. | |
| /// This is necessary as .NET does not provide managed alternatives for these calls. | |
| /// </summary> | |
| private static class NativeMethods | |
| { | |
| [DllImport("user32.dll")] | |
| public static extern IntPtr GetForegroundWindow(); | |
| [DllImport("user32.dll", SetLastError = true)] | |
| public static extern uint GetWindowThreadProcessId(IntPtr hWnd, out int lpdwProcessId); | |
| } |
| var threadProcessId = GetWindowThreadProcessId(window, out var processId); | ||
| return processId; | ||
| } | ||
|
|
||
| [DllImport("user32.dll")] | ||
| private static extern IntPtr GetForegroundWindow(); | ||
|
|
||
| [DllImport("user32.dll", SetLastError = true)] | ||
| private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out int lpdwProcessId); | ||
| } |
Copilot
AI
Nov 18, 2025
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.
Minimise the use of unmanaged code.
| var threadProcessId = GetWindowThreadProcessId(window, out var processId); | |
| return processId; | |
| } | |
| [DllImport("user32.dll")] | |
| private static extern IntPtr GetForegroundWindow(); | |
| [DllImport("user32.dll", SetLastError = true)] | |
| private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out int lpdwProcessId); | |
| } | |
| if (window == IntPtr.Zero) | |
| return 0; | |
| // Use managed code to find the process with this window handle | |
| var process = System.Diagnostics.Process.GetProcesses() | |
| .FirstOrDefault(p => p.MainWindowHandle == window); | |
| return process?.Id ?? 0; | |
| } | |
| [DllImport("user32.dll")] | |
| private static extern IntPtr GetForegroundWindow(); |
Fixes #2936
Add new builtin shortcut
{active_office_file}to quickly access the active office file.GetActiveOfficeFilePathinFlow.Launcher.Infrastructure/Helper.csto retrieve the active office file path.{active_office_file}inFlow.Launcher.Infrastructure/UserSettings/Settings.csto use theGetActiveOfficeFilePathmethod.BuiltinShortcutslist inFlow.Launcher.Infrastructure/UserSettings/Settings.csto include the new{active_office_file}shortcut.For more details, open the Copilot Workspace session.