feat: add show hidden files toggle to Project Explorer#9532
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @etherman-os on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. I completed the review and posted feedback on this pull request. Comment I'm re-reviewing this pull request in response to a review request. I completed the review and posted feedback on this pull request. Comment I reviewed this pull request and requested human review from: @moirahuang. Comment I completed the review and posted feedback on this pull request. Comment You can view the conversation on Warp. I reviewed this pull request and requested human review from: @moirahuang. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds a persisted Project Explorer setting, keyboard shortcut, header toggle, and file-tree flattening filter for hidden dotfiles.
Concerns
- The file tree unsubscribes from
CodeSettingswhile inactive, so toggling the setting while Project Explorer is closed can leave the cachedshow_hidden_filesvalue stale on the next activation. - The dotfile filter runs on the root entry too, so opening a hidden directory such as
~/.configas the project root can make Project Explorer render no root at all with the default setting. - Security pass: no additional security concerns found in the inlined diff.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| if is_active { | ||
| self.subscribe_to_repository_metadata(ctx); | ||
| self.subscribe_to_active_file_model(ctx); | ||
| self.subscribe_to_code_settings(ctx); |
There was a problem hiding this comment.
show_hidden_files stale and the activation rebuild below uses the old value. Refresh the cached setting before rebuilding.
| self.subscribe_to_code_settings(ctx); | |
| self.subscribe_to_code_settings(ctx); | |
| self.show_hidden_files = *CodeSettings::as_ref(ctx).show_hidden_files; |
| } | ||
|
|
||
| // Filter hidden files/directories when show_hidden_files is disabled. | ||
| if !self.show_hidden_files { |
There was a problem hiding this comment.
~/.config), leaving Project Explorer empty with the default setting. Apply the dotfile filter only to descendants.
| if !self.show_hidden_files { | |
| if !self.show_hidden_files && depth > 0 { |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
I'm checking this implementation PR for association with a likely matching ready issue. Powered by Oz |
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds a persisted setting, UI control, keybinding, and file-tree filtering path for showing hidden dotfiles in Project Explorer.
Concerns
- The hidden-file predicate calls
starts_withdirectly on anOsStr, which does not compile. - Toggling hidden files while a hidden item is selected can leave
selected_itempointing at a stale index after the rebuild. - The header icon reads the setting during render but does not appear to be notified for
ShowHiddenFileschanges, so shortcut/settings toggles can leave the icon state stale. - Supplemental security pass: no exploitable security issue was identified in the changed lines; showing dotfiles can expose sensitive-looking files only after an explicit user toggle/action.
Verdict
Found: 1 critical, 1 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| if let Some(name) = current_path.file_name() { | ||
| if name.starts_with('.') { | ||
| return (selected_item_index, removed_item); | ||
| } | ||
| } |
There was a problem hiding this comment.
🚨 [CRITICAL] file_name() returns an OsStr, which does not have starts_with('.'); this breaks compilation.
| if let Some(name) = current_path.file_name() { | |
| if name.starts_with('.') { | |
| return (selected_item_index, removed_item); | |
| } | |
| } | |
| if current_path | |
| .file_name() | |
| .is_some_and(|name| name.as_encoded_bytes().starts_with(b".")) | |
| { | |
| return (selected_item_index, removed_item); | |
| } |
| ctx.subscribe_to_model(&CodeSettings::handle(ctx), |me, _, event, ctx| { | ||
| if let CodeSettingsChangedEvent::ShowHiddenFiles { .. } = event { | ||
| me.show_hidden_files = *CodeSettings::as_ref(ctx).show_hidden_files; | ||
| me.rebuild_flattened_items(); |
There was a problem hiding this comment.
rebuild_flattened_items() returns no replacement index and leaves selected_item pointing at the old index, so keyboard actions can target a different visible file; clear or reselect after filtering removes the preserved path.
|
|
||
| // Show hidden files toggle when Project Explorer is active | ||
| if self.active_view() == ToolPanelView::ProjectExplorer { | ||
| let is_active = *CodeSettings::as_ref(app).show_hidden_files; |
There was a problem hiding this comment.
💡 [SUGGESTION] LeftPanelView reads show_hidden_files during render but does not subscribe/notify on ShowHiddenFiles, so toggling via the shortcut or settings can leave the eye icon stale until another panel render; add a notification path for this settings event.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds a persisted Project Explorer setting, a header toggle, a keyboard shortcut, and file-tree filtering for dotfile-style hidden files.
Concerns
- No blocking correctness or security issues found in the changed diff.
- Suggestions: keep the new button's MouseStateHandle stable, surface settings persistence failures, and scope the shortcut to the Project Explorer feature context.
Verdict
Found: 0 critical, 0 important, 3 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| ctx.subscribe_to_model(&CodeSettings::handle(ctx), |me, _, event, ctx| { | ||
| if let crate::settings::CodeSettingsChangedEvent::ShowHiddenFiles { .. } = event { | ||
| me.mouse_state_handles.hidden_files_toggle = MouseStateHandle::default(); |
There was a problem hiding this comment.
💡 [SUGGESTION] MouseStateHandle fields should be created once and reused; this setting change only needs ctx.notify() so the icon re-renders with the updated active state.
| WorkspaceAction::ToggleHiddenFiles, | ||
| ) | ||
| .with_group(bindings::BindingGroup::Navigation.as_str()) | ||
| .with_context_predicate(id!("Workspace")) |
There was a problem hiding this comment.
💡 [SUGGESTION] Scope this shortcut to the Project Explorer feature context so it does not toggle a Project Explorer-only setting when that panel is disabled.
| .with_context_predicate(id!("Workspace")) | |
| .with_context_predicate(id!("Workspace") & id!(flags::SHOW_PROJECT_EXPLORER)) |
| } | ||
| ToggleHiddenFiles => { | ||
| CodeSettings::handle(ctx).update(ctx, |settings, ctx| { | ||
| let _ = settings.show_hidden_files.toggle_and_save_value(ctx); |
There was a problem hiding this comment.
💡 [SUGGESTION] Do not discard persistence errors; use the existing reporting helper like nearby settings toggles.
| let _ = settings.show_hidden_files.toggle_and_save_value(ctx); | |
| report_if_error!(settings.show_hidden_files.toggle_and_save_value(ctx)); |
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds a persisted Project Explorer setting, header toggle, keybinding, and file-tree filtering path for showing or hiding dotfiles.
Concerns
- The new hidden-file predicate calls
as_bytes()on anOsStr, which is not an inherent cross-platform method and is not imported from a Unix-only extension trait, so this change will fail to compile as written. - Supplemental security pass found no security-specific findings in the changed lines.
Verdict
Found: 1 critical, 0 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| if current_path | ||
| .file_name() | ||
| .is_some_and(|name| name.as_bytes().starts_with(b".")) |
There was a problem hiding this comment.
🚨 [CRITICAL] OsStr::as_bytes is not an inherent cross-platform method, so this fails to compile without a Unix-only trait import and would still break Windows builds. Use a portable filename check.
| if current_path | |
| .file_name() | |
| .is_some_and(|name| name.as_bytes().starts_with(b".")) | |
| if current_path | |
| .file_name() | |
| .is_some_and(|name| name.to_string_lossy().starts_with('.')) |
|
/oz-review |
There was a problem hiding this comment.
Overview
Adds a persisted CodeSettings toggle for showing dotfiles in the Project Explorer, wires it to file-tree flattening, a header button, and a keyboard shortcut.
Concerns
- No blocking correctness or security concerns found in the inlined diff.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
captainsafia
added
the
external-contributor
Warp Dev Github Integration
|
@etherman-os thank you for the PR! there's a related issue #7502 that needs design input. i've added the |
|
hey @etherman-os thanks so much for your work on this! I've added design mocks to the related issue here: #7502 (comment) |
|
Aligned the Project Explorer hidden-files toggle with the new mocks: the header button now switches between eye / eye-off states and uses state-specific tooltips ( |
|
/oz-review |
moirahuang
left a comment
moirahuang
left a comment
There was a problem hiding this comment.
a higher level comment: i just met with @eluzzi5 and we're going to update the mocks so we don't show the eye icon in the project explorer but instead have the setting in the settings file + the editable keyboard shortcut. she'll update the mocks to reflect that!
| .with_enabled(|| FeatureFlag::GlobalSearch.is_enabled()) | ||
| .with_custom_action(CustomAction::ToggleGlobalSearch), | ||
| EditableBinding::new( | ||
| "file_tree:toggle_hidden_files", |
There was a problem hiding this comment.
i tried to locally test this and it wasn't working, mind recording a video of you testing the keyboard shortcut please?
etherman-os
force-pushed
the
feat/show-hidden-files
branch
from
1975a64 to
ff01610
Compare
|
Thanks for the heads up. I rebuilt this on current I found the Linux/Windows default was registered as I also removed the Project Explorer header eye icon per the updated design direction, leaving this as a settings-file value + editable keyboard shortcut. Local verification video: https://raw.githubusercontent.com/etherman-os/warp/pr-9532-assets/pr-assets/9532/pr9532-hidden-files-shortcut.mp4 Verified:
|
I believe we have the same issue for Mac, we need to update |
|
@etherman-os I just realized that the proposed Windows / Linux keyboard shortcut conflicts with our existing keyboard shortcuts for increasing and decreasing font size. I just met with @eluzzi5 and because In this PR, mind making the update for font zoom in:
and for font zoom out:
And for hide / show hidden files we'll keep:
Thank you! |
|
Updated based on the latest shortcut direction. Changes pushed:
Verified:
|
|
@etherman-os sorry I realized I never merged this. I'm going to ask @oz-agent to resolve the merge conflicts and try to get this in! @oz-agent please resolve the merge conflicts. If there is any ambiguity, don't force it and instead surface the areas of uncertainty as a comment |
|
I'm working on changes requested in this PR (responding to a PR conversation comment). You can view the conversation on Warp. I pushed changes to this PR based on the comment. Next steps:
Powered by Oz |
Move the overlapping imports so the hidden-files branch can merge cleanly with current master while preserving the existing Project Explorer behavior. Co-Authored-By: Moira Huang <moirahuang@users.noreply.github.com>
|
@oz-agent please fix the formatting errors and push the change |
|
I'm working on changes requested in this PR (responding to a PR conversation comment). You can view the conversation on Warp. I pushed changes to this PR based on the comment. Next steps:
Powered by Oz |
Co-Authored-By: Oz <oz-agent@warp.dev> Co-Authored-By: Moira Huang <moirahuang@users.noreply.github.com>
|
@oz-agent please fix the merge conflicts and push the fix |
|
I'm working on changes requested in this PR (responding to a PR conversation comment). You can view the conversation on Warp. I pushed changes to this PR based on the comment. Next steps:
Powered by Oz |
Fully qualify hidden-files test setting helpers so the import block can auto-merge with current master. Co-Authored-By: Oz <oz-agent@warp.dev> Co-Authored-By: Moira Huang <moirahuang@users.noreply.github.com>
5 participants
Summary
Changes
show_hidden_filesinCodeSettings(default:false, persisted to TOML)flatten_entry_for_rootwhen setting is disabledCtrl+Shift+.(Linux/Windows),Cmd+Shift+.(Mac)Implementation Details
Files Changed
app/src/settings/code.rsshow_hidden_filessetting definitionapp/src/code/file_tree/view.rsapp/src/workspace/view/left_panel.rsapp/src/workspace/action.rsToggleHiddenFilesaction variantapp/src/workspace/mod.rsapp/src/workspace/view.rsDesign Decisions
define_settings_group!macro to~/.config/warp-terminal/settings.toml.gitdirectory: Already handled byis_git_internal_path, unaffected by this togglefile_name()returningNoneis handled — files without names are shownNotes
cargo checkandcargo clippy— no errorsTesting
Ctrl+Shift+..gitignore,.env,.config/, etc.) appear/disappear.gitdirectory behavior is unchanged