-
-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,14 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #nullable enable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #nullable enable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.IO; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Text.Json; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Text.Json.Serialization; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Diagnostics; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Runtime.InteropServices; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Linq; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Collections.Generic; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using win32api = Microsoft.Win32; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace Flow.Launcher.Infrastructure | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -86,5 +91,41 @@ public static string Formatted<T>(this T t) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return formatted; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | |
| /// Gets the file path of the currently active Microsoft Office document (Word, Excel, or PowerPoint). | |
| /// </summary> | |
| /// <returns> | |
| /// The full path to the active Office file, or <c>null</c> if no supported Office application is in focus or no document is active. | |
| /// </returns> |
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);
}
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;
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)) |
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.
Microsoft.Win32.Dispatch does not exist. For Office COM automation, you should use dynamic with Type.GetTypeFromProgID() and Activator.CreateInstance(), similar to the pattern in FileExplorerHelper.cs:
Type wordType = Type.GetTypeFromProgID("Word.Application");
if (wordType == null) return null;
dynamic wordApp = Activator.CreateInstance(wordType);
dynamic activeDoc = wordApp?.ActiveDocument;
return activeDoc?.FullName != null ? Path.GetFullPath(activeDoc.FullName) : null;Additionally, this code lacks error handling. Office applications might not have an active document (e.g., if Word is open but no document is loaded), which would throw a COM exception.
Apply similar fixes to lines 106 and 110 for PowerPoint and Excel.
| 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); | |
| try | |
| { | |
| Type wordType = Type.GetTypeFromProgID("Word.Application"); | |
| if (wordType == null) return null; | |
| dynamic wordApp = Activator.CreateInstance(wordType); | |
| dynamic activeDoc = wordApp?.ActiveDocument; | |
| return activeDoc?.FullName != null ? Path.GetFullPath(activeDoc.FullName) : null; | |
| } | |
| catch | |
| { | |
| return null; | |
| } | |
| } | |
| else if (exePath.ToLower().Contains("powerpnt.exe")) | |
| { | |
| try | |
| { | |
| Type pptType = Type.GetTypeFromProgID("PowerPoint.Application"); | |
| if (pptType == null) return null; | |
| dynamic pptApp = Activator.CreateInstance(pptType); | |
| dynamic activePres = pptApp?.ActivePresentation; | |
| return activePres?.FullName != null ? Path.GetFullPath(activePres.FullName) : null; | |
| } | |
| catch | |
| { | |
| return null; | |
| } | |
| } | |
| else if (exePath.ToLower().Contains("excel.exe")) | |
| { | |
| try | |
| { | |
| Type excelType = Type.GetTypeFromProgID("Excel.Application"); | |
| if (excelType == null) return null; | |
| dynamic excelApp = Activator.CreateInstance(excelType); | |
| dynamic activeWb = excelApp?.ActiveWorkbook; | |
| return activeWb?.FullName != null ? Path.GetFullPath(activeWb.FullName) : null; | |
| } | |
| catch | |
| { | |
| 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.
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) | |
| { |
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.
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.
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 threadProcessId variable on line 121 is assigned but never used. You can simplify this to:
private static int GetActiveWindowProcessId()
{
var window = GetForegroundWindow();
GetWindowThreadProcessId(window, out var processId);
return processId;
}| var threadProcessId = GetWindowThreadProcessId(window, out var processId); | |
| 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.
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); | |
| } |
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(); |
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 usedRemove unused imports to keep the code clean.