Add ability to edit in Photoshop and copy path of current image to clipboard#10
Add ability to edit in Photoshop and copy path of current image to clipboard#10AlanRockefeller merged 3 commits intomainfrom
Conversation
This commit introduces new features: - Pressing 'E' now opens the current image in Adobe Photoshop. - Pressing 'Ctrl+C' copies the path of the current image to the clipboard. - The application's status bar now displays messages for actions like copying to clipboard or launching Photoshop. Changes include: - Updated aststack/faststack/config.py to include a 'photoshop' section for executable path and arguments. - Modified aststack/faststack/ui/keystrokes.py to map 'E' to 'edit_in_photoshop' and 'Ctrl+C' to 'copy_path_to_clipboard'. - Implemented edit_in_photoshop, copy_path_to_clipboard, and update_status_message methods in aststack/faststack/app.py. - Enhanced aststack/faststack/ui/provider.py by adding statusMessageChanged signal and statusMessage property to UIState. - Updated aststack/faststack/qml/Main.qml to display the uiState.statusMessage in the footer. - Removed the unused aststack/faststack/ui/state.py file.
Replaced the lambda function in update_status_message with a nested function to avoid a SyntaxError caused by an assignment within the lambda.
This commit addresses two UI issues: 1. The Key Bindings dialog now uses a ScrollView to ensure all text is visible, and the key bindings list has been updated. 2. The Settings dialog now includes a field for selecting the Photoshop executable path. Changes include: - Updated aststack/faststack/qml/Main.qml to wrap the Key Bindings text in a ScrollView and to load the Photoshop path into the Settings dialog. - Modified aststack/faststack/qml/SettingsDialog.qml to include UI elements for Photoshop path selection. - Added get_photoshop_path and set_photoshop_path methods to aststack/faststack/app.py. - Exposed the new methods to QML through aststack/faststack/ui/provider.py.
WalkthroughAdds filtering capability with apply/clear filter slots, drag-and-drop support via Windows HDROP, Photoshop and clipboard integration, status messaging, and improved UI state synchronization through new signals and properties across controller, configuration, QML, provider, and keystroke modules. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant QML as QML UI
participant Ctrl as AppController
participant State as UIState
participant Model as Image Model
User->>QML: Opens Filter Dialog
User->>QML: Enters filter string, clicks OK
QML->>Ctrl: apply_filter(filter_string)
Ctrl->>Ctrl: refresh_image_list() with active filter
Ctrl->>Model: Update image_files
Ctrl->>State: emit dataChanged()
Ctrl->>Ctrl: Reset current index, sync_ui_state()
State->>QML: dataChanged signal
QML->>QML: Update image list display
User->>QML: Clicks "Clear Filename Filter"
QML->>Ctrl: clear_filter()
Ctrl->>Ctrl: Disable filter
Ctrl->>Ctrl: refresh_image_list() (unfiltered)
Ctrl->>Model: Update image_files (all)
Ctrl->>State: emit dataChanged()
Ctrl->>Ctrl: Adjust index, sync_ui_state()
State->>QML: dataChanged signal
QML->>QML: Display all images
sequenceDiagram
participant User as User
participant QML as QML UI
participant Ctrl as AppController
participant Config as Config
participant Sys as System/Photoshop
User->>QML: Opens Settings Dialog
QML->>QML: Displays current Photoshop path
User->>QML: Clicks Browse for Photoshop path
QML->>Ctrl: open_file_dialog()
Ctrl->>Sys: File picker dialog
Sys->>QML: Returns selected path
QML->>QML: Updates photoshopPath field
QML->>Ctrl: check_path_exists(path)
QML->>QML: Shows checkmark if valid
User->>QML: Clicks OK in Settings
QML->>Ctrl: set_photoshop_path(path)
Ctrl->>Config: Persist Photoshop exe path
User->>QML: Selects image, presses E
QML->>Ctrl: edit_in_photoshop()
Ctrl->>Config: Retrieve Photoshop exe and args
Ctrl->>Sys: Launch Photoshop with image file
Ctrl->>QML: update_status_message("Opening in Photoshop...")
QML->>User: Display status message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
faststack/faststack/qml/SettingsDialog.qml (1)
56-78: Consider extracting a reusable component for path configuration.The Photoshop Path section (lines 56-78) duplicates the pattern from the Helicon Path section (lines 32-54). Consider creating a reusable QML component to reduce duplication.
Example structure:
// PathConfigRow.qml GridLayout { property string label: "" property string value: "" property var onValueChanged: function(v) {} Label { text: label } TextField { Layout.fillWidth: true text: value onTextChanged: onValueChanged(text) } RowLayout { Button { text: "Browse..." onClicked: { var path = uiState.open_file_dialog() if (path) value = path } } Label { text: "✔" color: "lightgreen" visible: uiState.check_path_exists(value) } } }faststack/faststack/ui/provider.py (1)
42-44: Consider usinglogging.exceptionfor error logging.When logging within an exception handler,
logging.exceptionautomatically includes the traceback, which aids debugging.except (ValueError, IndexError) as e: - log.error(f"Invalid image ID requested from QML: {id}. Error: {e}") + log.exception(f"Invalid image ID requested from QML: {id}")Based on static analysis hints.
faststack/faststack/app.py (1)
443-444: Commented-out code should be removed.The
os.remove(tmp_path)line is commented out with a note to keep the temporary file. Either remove the commented code or document why the temporary file should be kept.- # os.remove(tmp_path) - log.info(f"Keeping temporary file: {tmp_path}") + log.debug(f"Temporary file retained for Helicon: {tmp_path}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
faststack/faststack/app.py(15 hunks)faststack/faststack/config.py(1 hunks)faststack/faststack/qml/Main.qml(7 hunks)faststack/faststack/qml/SettingsDialog.qml(3 hunks)faststack/faststack/ui/keystrokes.py(1 hunks)faststack/faststack/ui/provider.py(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
faststack/faststack/app.py (7)
faststack/faststack/ui/provider.py (16)
ImageProvider(13-45)UIState(48-260)theme(73-74)theme(77-82)get_theme(233-235)currentIndex(116-117)imageCount(120-121)currentFilename(128-129)isFlagged(132-133)isRejected(136-137)stackInfoText(148-149)get_photoshop_path(201-202)set_photoshop_path(205-206)statusMessage(162-163)statusMessage(166-169)get_stack_summary(152-159)faststack/faststack/ui/keystrokes.py (1)
Keybinder(7-88)faststack/faststack/imaging/prefetch.py (1)
update_prefetch(36-56)faststack/faststack/io/indexer.py (1)
find_images(19-54)faststack/faststack/io/sidecar.py (2)
get_metadata(85-87)save(54-83)faststack/faststack/config.py (2)
save(53-61)get(63-64)faststack/faststack/io/watcher.py (1)
start(49-62)
faststack/faststack/ui/provider.py (2)
faststack/faststack/models.py (1)
DecodedImage(33-42)faststack/faststack/app.py (4)
get_photoshop_path(464-465)set_photoshop_path(467-469)get_theme(498-499)set_theme(501-511)
🪛 Ruff (0.14.4)
faststack/faststack/app.py
40-40: Redefinition of unused ImageProvider from line 14
Remove definition: ImageProvider
(F811)
607-607: subprocess call: check for execution of untrusted input
(S603)
610-610: Do not catch blind exception: Exception
(BLE001)
612-612: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
670-670: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
faststack/faststack/ui/provider.py
43-43: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (21)
faststack/faststack/ui/keystrokes.py (4)
8-15: LGTM! Clean refactoring to support flexible dispatch.The change from
main_windowtocontrollerwith the dispatch pattern allows for better separation of concerns and makes the keybinding system more flexible.
48-61: LGTM! Proper fallback chain for method dispatch.The
_callmethod correctly implements a fallback chain: QML root first (for UI sync), then controller, with appropriate logging for missing methods.
81-86: Good fallback for non-standard keyboard layouts.The text-based bracket key fallback is a thoughtful addition for handling keyboards where bracket keycodes might not match
Qt.Key_BracketLeft/Right.
69-72: The original review comment is based on a misunderstanding of Qt's modifier key API and can be disregarded.The bitwise AND operation is the correct and standard pattern for checking modifier keys in Qt/PyQt. In Qt,
event.modifiers()returns a combined set of all currently pressed modifiers. The bitwise AND check (event.modifiers() & mapped_modifier) correctly tests whether a specific modifier flag is set, even when other modifiers are also pressed. Using strict equality (==) would actually break the code—for example, if the user pressed Ctrl+Shift, the equality check would fail becauseevent.modifiers()would represent both modifiers combined. The current implementation at line 70 is correct and follows Qt best practices.Likely an incorrect or invalid review comment.
faststack/faststack/ui/provider.py (3)
71-82: LGTM! Proper Qt property implementation for theme.The theme property correctly implements the Qt property pattern with getter, setter, and change notification signal.
161-169: LGTM! Status message property follows Qt patterns.The statusMessage property is well-implemented with proper change detection and signal emission.
200-206: LGTM! Consistent delegation pattern for Photoshop path.The get/set_photoshop_path slots follow the same pattern as the existing helicon_path methods, maintaining consistency.
faststack/faststack/qml/Main.qml (4)
18-34: LGTM! Improved theme binding with reactive properties.Migrating from
get_theme()method calls touiState.themeproperty binding enables reactive updates and eliminates manual synchronization.
112-118: LGTM! Clean status message display implementation.The status message label with conditional visibility provides good user feedback without cluttering the UI when empty.
250-276: LGTM! Comprehensive keyboard shortcuts documentation.The about dialog now documents all keyboard shortcuts including the new Photoshop (E) and clipboard (Ctrl+C) actions.
186-187: FilterDialog component verified and properly implemented.The FilterDialog component is correctly defined inline at lines 303-307 in Main.qml with the id
filterDialogand includes the necessaryonAcceptedsignal handler. The reference at line 186 (filterDialog.open()) is valid and the component is properly accessible.faststack/faststack/app.py (9)
44-58: LGTM! Correct Windows HDROP implementation.The
make_hdropfunction properly constructs a CF_HDROP payload with the DROPFILES header structure required for Windows drag-and-drop operations.
63-63: LGTM! General data change signal for UI synchronization.Adding a
dataChangedsignal provides a way to notify the UI of broad state changes without multiple specific signals.
120-153: LGTM! Well-structured filtering implementation.The filtering workflow correctly:
- Validates input
- Updates internal state
- Refreshes the image list
- Emits signals
- Resets navigation
- Triggers prefetch
218-228: LGTM! Filter applied during image list refresh.The filter is correctly applied during
refresh_image_list, ensuring consistency whenever the image list is rebuilt.
464-469: LGTM! Consistent pattern for Photoshop path configuration.The get/set methods follow the same pattern as the existing Helicon path methods.
501-511: LGTM! Proper theme state synchronization.The method now updates both the UI state property and persists the configuration, ensuring consistency across the application.
615-623: LGTM! Clean clipboard integration.The clipboard copy functionality is straightforward and includes appropriate user feedback via status messages.
625-634: LGTM! Useful status message helper with timeout.The status message helper with automatic clearing provides good UX without cluttering the interface.
638-677: LGTM! Comprehensive drag-and-drop implementation.The drag-and-drop implementation:
- Validates file existence
- Handles Windows-specific HDROP format
- Falls back to standard URLs for other platforms
- Includes a thumbnail preview
- Logs appropriately
faststack/faststack/config.py (1)
22-25: Remove the concern—"Photoshop 2026" is the current version as of November 2025.The latest Adobe Photoshop desktop version is 27.0 (Photoshop 2026), and the path C:\Program Files\Adobe\Adobe Photoshop 2026\ matches Adobe's documented typical Windows installation path format. The hardcoded version in the config is accurate and aligns with the current target version.
| from faststack.imaging.cache import ByteLRUCache, get_decoded_image_size | ||
| from faststack.imaging.prefetch import Prefetcher | ||
| from faststack.ui.provider import ImageProvider, UIState | ||
| from faststack.ui.provider import ImageProvider |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate import.
ImageProvider is imported twice: once on line 14 and again on line 40.
from faststack.imaging.prefetch import Prefetcher
-from faststack.ui.provider import ImageProvider
from faststack.ui.keystrokes import KeybinderBased on static analysis hints.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from faststack.ui.provider import ImageProvider | |
| from faststack.imaging.prefetch import Prefetcher | |
| from faststack.ui.keystrokes import Keybinder |
🧰 Tools
🪛 Ruff (0.14.4)
40-40: Redefinition of unused ImageProvider from line 14
Remove definition: ImageProvider
(F811)
🤖 Prompt for AI Agents
In faststack/faststack/app.py around line 40, there is a duplicate import of
ImageProvider (already imported on line 14); remove the redundant import
statement at line 40 so ImageProvider is imported only once, then run the
linter/tests to confirm no unused-import or import-order issues remain.
| @Slot() | ||
| def edit_in_photoshop(self): | ||
| if not self.image_files: | ||
| self.update_status_message("No image to edit.") | ||
| return | ||
|
|
||
| current_image_path = self.image_files[self.current_index].path | ||
| photoshop_exe = config.get('photoshop', 'exe') | ||
| photoshop_args = config.get('photoshop', 'args') | ||
|
|
||
| if not photoshop_exe or not Path(photoshop_exe).exists(): | ||
| self.update_status_message("Photoshop executable not configured or not found.") | ||
| log.error(f"Photoshop executable not found: {photoshop_exe}") | ||
| return | ||
|
|
||
| try: | ||
| command = [photoshop_exe] | ||
| if photoshop_args: | ||
| command.extend(photoshop_args.split()) | ||
| command.append(str(current_image_path)) | ||
|
|
||
| subprocess.Popen(command) | ||
| self.update_status_message(f"Opened {current_image_path.name} in Photoshop.") | ||
| log.info(f"Launched Photoshop with: {command}") | ||
| except Exception as e: | ||
| self.update_status_message(f"Failed to open in Photoshop: {e}") | ||
| log.error(f"Error launching Photoshop: {e}") |
There was a problem hiding this comment.
Security: Validate Photoshop executable path before execution.
The code executes the configured Photoshop path without validation. Consider adding signature verification or restricting to known installation paths.
Also, replace the broad Exception catch with specific exception types, and use logging.exception to include traceback:
try:
command = [photoshop_exe]
if photoshop_args:
command.extend(photoshop_args.split())
command.append(str(current_image_path))
subprocess.Popen(command)
self.update_status_message(f"Opened {current_image_path.name} in Photoshop.")
log.info(f"Launched Photoshop with: {command}")
- except Exception as e:
+ except (OSError, subprocess.SubprocessError) as e:
self.update_status_message(f"Failed to open in Photoshop: {e}")
- log.error(f"Error launching Photoshop: {e}")
+ log.exception("Error launching Photoshop")Based on static analysis hints.
🧰 Tools
🪛 Ruff (0.14.4)
607-607: subprocess call: check for execution of untrusted input
(S603)
610-610: Do not catch blind exception: Exception
(BLE001)
612-612: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
|
@coderabbitai What improvements could be made? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 28381 🏁 Script executed: Length of output: 2797 🏁 Script executed: Length of output: 2839 🏁 Script executed: Length of output: 8207 🏁 Script executed: Length of output: 123 🏁 Script executed: Length of output: 10448 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 156 🏁 Script executed: Length of output: 123 🏁 Script executed: Length of output: 32 🏁 Script executed: Length of output: 26 🏁 Script executed: Length of output: 306 🏁 Script executed: Length of output: 19 🏁 Script executed: Length of output: 55 🏁 Script executed: Length of output: 1711 🏁 Script executed: Length of output: 2949 🏁 Script executed: Length of output: 0 @AlanRockefeller I've analyzed your PR and identified several areas for improvement. Here are my recommendations organized by priority: 🚨 Critical Issues1. Missing FilterDialog.qml ComponentMain.qml references FilterDialog {
id: filterDialog
onAccepted: {
controller.apply_filter(filterString)
}
}Fix: Create 2. Property Name MismatchMain.qml uses # In provider.py, should be:
@Property(str, notify=statusMessageChanged)
def stackSummary(self): # Changed from get_stack_summary
# ... implementation
|
1 participant
Summary by CodeRabbit
Release Notes
New Features
UI Improvements