fix: add margin to file thumbnails in file explorer window - #1338
fix: add margin to file thumbnails in file explorer window#1338joey-wheeler99 wants to merge 1 commit into
Conversation
WalkthroughThe filesystem dock now applies a horizontal separation of ChangesFilesystem thumbnail spacing
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@editor/docks/filesystem_dock.cpp`:
- Line 970: Update the h_separation theme constant override in the
thumbnail-mode setup to multiply the value 10 by EDSCALE, matching the existing
scaling used for thumbnail_size.
- Line 970: Remove or reset the files control’s h_separation theme override in
the non-thumbnail/list-mode branch, alongside the existing thumbnail setup, so
switching modes after files->clear() restores the intended list spacing. Update
the mode-switching logic around the files control without changing
thumbnail-mode spacing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3306c194-11e1-446b-9af1-582a58e105c0
📒 Files selected for processing (1)
editor/docks/filesystem_dock.cpp
| files->set_fixed_column_width(thumbnail_size * 3 / 2); | ||
| files->set_max_text_lines(2); | ||
| files->set_fixed_icon_size(Size2(thumbnail_size, thumbnail_size)); | ||
| files->add_theme_constant_override("h_separation", 10); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'filesystem_dock\.cpp|editor_profiler\.cpp' . | sed 's#^\./##'
echo "== relevant filesystem_dock section =="
if [ -f editor/docks/filesystem_dock.cpp ]; then
nl -ba editor/docks/filesystem_dock.cpp | sed -n '930,1005p'
fi
echo "== search theme constant overrides and EDSCALE usage =="
rg -n "add_theme_constant_override|\"h_separation\"|EDSCALE|thumbnail_size" editor/docks/filesystem_dock.cpp editor/debugger/editor_profiler.cpp 2>/dev/null || true
echo "== nearby profiler snippet =="
if [ -f editor/debugger/editor_profiler.cpp ]; then
nl -ba editor/debugger/editor_profiler.cpp | rg -n "h_separation|thumbnail_size|EDSCALE" -C 3
fiRepository: Redot-Engine/redot-engine
Length of output: 368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'filesystem_dock\.cpp|editor_profiler\.cp' . | sed 's#^\./##' || true
echo "== relevant filesystem_dock section =="
if [ -f editor/docks/filesystem_dock.cpp ]; then
nl -ba editor/docks/filesystem_dock.cpp | sed -n '930,1005p'
fi
echo "== search theme constant overrides and EDSCALE usage =="
rg -n "add_theme_constant_override|\"h_separation\"|EDSCALE|thumbnail_size" editor/docks/filesystem_dock.cpp editor/debugger/editor_profiler.cpp 2>/dev/null || true
echo "== nearby profiler snippet =="
if [ -f editor/debugger/editor_profiler.cpp ]; then
nl -ba editor/debugger/editor_profiler.cpp | rg -n "h_separation|thumbnail_size|EDSCALE" -C 3 || true
fiRepository: Redot-Engine/redot-engine
Length of output: 368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant filesystem_dock section =="
awk 'NR>=930 && NR<=1005 {printf "%6d\t%s\n", NR, $0}' editor/docks/filesystem_dock.cpp
echo "== search theme constant overrides and EDSCALE usage =="
grep -nE 'add_theme_constant_override|"[^"]*h_separation[^"]*"|EDSCALE|thumbnail_size' editor/docks/filesystem_dock.cpp editor/debugger/editor_profiler.cpp 2>/dev/null || true
echo "== profiler relevant lines =="
grep -nE 'h_separation|thumbnail_size|EDSCALE' editor/debugger/editor_profiler.cpp 2>/dev/null -C 2 || trueRepository: Redot-Engine/redot-engine
Length of output: 8184
Scale h_separation with EDSCALE.
thumbnail_size is computed with EDSCALE, but the thumbnail-mode h_separation override remains 10. Multiply this override by EDSCALE so the separation scales with the thumbnails.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@editor/docks/filesystem_dock.cpp` at line 970, Update the h_separation theme
constant override in the thumbnail-mode setup to multiply the value 10 by
EDSCALE, matching the existing scaling used for thumbnail_size.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)filesystem_dock\.cpp$|(^|/)theme_override|local_theme_override' || true
echo
echo "filesystem_dock.cpp around relevant lines:"
sed -n '900,990p' editor/docks/filesystem_dock.cpp
echo
echo "Search h_separation/theme overrides in filesystem_dock.cpp:"
rg -n 'h_separation|theme_constant_override|clear\(\)' editor/docks/filesystem_dock.cpp
echo
echo "Nearby update_file_list outlines/functions:"
ast-grep outline editor/docks/filesystem_dock.cpp --match _update_file_list --view expanded || true
sed -n '840,930p' editor/docks/filesystem_dock.cppRepository: Redot-Engine/redot-engine
Length of output: 7281
🌐 Web query:
Godot Control remove_theme_constant_override docs Control local theme overrides clear remove_local_theme_override
💡 Result:
In Godot, local theme overrides for Control nodes are managed using specific methods provided by the Control class [1][2]. These overrides allow a node to ignore the values provided by its assigned Theme and use a custom, local value instead [3]. To remove a specific theme constant override that was previously set (either via code or the Inspector), you should use the remove_theme_constant_override method [2][4]. Method Signature: void remove_theme_constant_override(name: StringName) Functionality: The remove_theme_constant_override method removes the local override for a theme constant with the specified name [4]. Once removed, the Control will revert to using the value provided by its active Theme (or the default theme if no custom theme is assigned) [1][3]. Other similar methods exist for different theme data types, following the same naming convention [5][6]: - remove_theme_color_override(name: StringName) [2][5] - remove_theme_font_override(name: StringName) [5][6] - remove_theme_font_size_override(name: StringName) [5][6] - remove_theme_icon_override(name: StringName) [5][6] - remove_theme_style_override(name: StringName) [5][6] Note that these methods are distinct from the Theme class's own methods (such as clear_constant), which are used for modifying Theme resources themselves, rather than individual Control node overrides [7]. If you are working with an older version of Godot (prior to these methods being added), you would have had to manually reset values to their defaults [8], but in current Godot 4.x versions, these dedicated remove methods are the standard and recommended approach [2][4].
Citations:
- 1: https://docs.godotengine.org/en/stable/classes/class_control.html
- 2: https://docs.godotengine.org/en/4.4/classes/class_control.html
- 3: https://docs.godotengine.org/en/stable/tutorials/ui/gui%5Fskinning.html
- 4: https://docs.w3cub.com/godot~4.2/classes/class_window
- 5: https://github.com/godotengine/godot/blob/8dcf5b4e/scene/gui/control.h
- 6: https://github.com/godotengine/godot/blob/4a919adc/scene/main/window.h
- 7: https://docs.godotengine.org/en/stable/classes/class_theme.html
- 8: Not clear what "Theme Properties" are and how to retrieve and set them godotengine/godot-docs#2889
Do not leave the thumbnail override in list mode.
files->clear() does not remove theme constant overrides, so switching from thumbnails back to the list view reuses the same files control while keeping h_separation set. Remove the override in the non-thumbnail branch or restore the intended list-mode spacing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@editor/docks/filesystem_dock.cpp` at line 970, Remove or reset the files
control’s h_separation theme override in the non-thumbnail/list-mode branch,
alongside the existing thumbnail setup, so switching modes after files->clear()
restores the intended list spacing. Update the mode-switching logic around the
files control without changing thumbnail-mode spacing.
Shakai-Dev
left a comment
There was a problem hiding this comment.
Code LGTM but needs testing
This should fix #670
Summary by CodeRabbit