Conversation
very wip... - only connects to current view when activated, not 'any' view - does not handle multiple selection - image resolution no good (it's using thumbnails still) - pane sizing is problematic. - many image types not shown currently. - ...
|
Looking good |
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new preview pane feature to Nemo, allowing users to view image previews and file details directly within the file manager. The implementation includes a split-pane interface with image preview at the top and metadata at the bottom, activated via F7 or the View menu.
Key Changes:
- New preview pane infrastructure with split-pane layout (horizontal for main view, vertical for preview sections)
- Image preview widget with optimized loading and responsive resize handling
- File details widget displaying metadata (name, size, type, modified date, permissions, location)
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/nemo-window.h | Added public API declarations for preview pane on/off/showing functions |
| src/nemo-window.c | Implemented preview pane lifecycle management, selection tracking, and split view integration |
| src/nemo-window-private.h | Added preview pane widget and state flag to window details struct |
| src/nemo-window-menus.c | Added preview pane toggle action with F7 keyboard shortcut |
| src/nemo-preview-pane.h | Header for main preview pane container widget |
| src/nemo-preview-pane.c | Container managing vertical split between image and details widgets |
| src/nemo-actions.h | Added action constant for preview pane toggle |
| src/meson.build | Registered preview pane source file in build |
| libnemo-private/org.nemo.gschema.xml | Added settings schema for pane width and details height persistence |
| libnemo-private/nemo-preview-image.h | Header for image preview widget |
| libnemo-private/nemo-preview-image.c | Image display with debounced resize and Cairo rendering |
| libnemo-private/nemo-preview-details.h | Header for file metadata widget |
| libnemo-private/nemo-preview-details.c | Grid-based display of file properties |
| libnemo-private/nemo-global-preferences.h | Declared preview pane GSettings object |
| libnemo-private/nemo-global-preferences.c | Initialized and finalized preview pane settings |
| libnemo-private/meson.build | Registered preview image and details source files |
| gresources/nemo-shell-ui.xml | Added preview pane menu item to View menu |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Forward declaration for preview pane callback */ | ||
| static void preview_pane_selection_changed_callback (NemoView *view, NemoWindow *window); | ||
|
|
There was a problem hiding this comment.
Forward declaration placed far from its usage. Consider moving this declaration closer to the first function that uses it (nemo_window_connect_content_view at line 1587) or reorganizing the function order to eliminate the forward declaration.
| /* Forward declaration for preview pane callback */ | |
| static void preview_pane_selection_changed_callback (NemoView *view, NemoWindow *window); |
| /* Fallback to 60/40 split if no saved value */ | ||
| gtk_paned_set_position (GTK_PANED (window->details->split_view_hpane), | ||
| total_width * 0.6); |
There was a problem hiding this comment.
Magic number 0.6 should be defined as a named constant (e.g., DEFAULT_PREVIEW_PANE_RATIO) to clarify intent and enable easy adjustment.
| widget_height = gtk_widget_get_allocated_height (widget); | ||
|
|
||
| /* Get surface dimensions - works for image surfaces created from pixbufs */ | ||
| scale_factor = gtk_widget_get_scale_factor (widget); |
There was a problem hiding this comment.
Potential division by zero if scale_factor is 0. The gtk_widget_get_scale_factor documentation indicates it returns a positive integer, but defensive coding would add a check or assertion before division to handle unexpected values.
| scale_factor = gtk_widget_get_scale_factor (widget); | |
| scale_factor = gtk_widget_get_scale_factor (widget); | |
| if (scale_factor <= 0) { | |
| g_warning ("gtk_widget_get_scale_factor returned %d; avoiding division by zero.", scale_factor); | |
| return FALSE; | |
| } |
| ui_scale = gtk_widget_get_scale_factor (GTK_WIDGET (widget)); | ||
| orig_width = gdk_pixbuf_get_width (priv->current_pixbuf); | ||
| orig_height = gdk_pixbuf_get_height (priv->current_pixbuf); | ||
|
|
There was a problem hiding this comment.
Potential division by zero if orig_width or orig_height is 0. While gdk_pixbuf_get_width/height typically return positive values, a zero-dimension pixbuf could cause a crash. Add validation before performing division.
| /* Prevent division by zero if pixbuf has zero width or height */ | |
| if (orig_width == 0 || orig_height == 0) { | |
| return; | |
| } |
2 participants
ref: #2122, #3085, #2431
The nemo-preview extension isn't really good long-term to maintain, and is broken on some distros
Just targeting image types here for the moment.
todo: