Add setting for customizable delimiter for file drag-and-drop#19799
Add setting for customizable delimiter for file drag-and-drop#19799Vallabh-1504 wants to merge 2 commits intomicrosoft:mainfrom
Conversation
21e4b1a to
fb30903
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a new profile setting dragDropDelimiter that allows users to customize the delimiter used when dragging and dropping multiple files into the terminal. The default delimiter remains a single space for backward compatibility, but users can now configure it to use comma-space, semicolon, or any custom string to match their shell's syntax requirements (e.g., PowerShell's comma-separated paths).
Changes:
- Added
dragDropDelimiterprofile setting to the settings model, control properties, and UI with a default value of" "(single space) - Updated drag-and-drop logic in TermControl to use the configurable delimiter instead of hardcoded space
- Added settings UI in the Advanced tab with localized strings for the delimiter configuration
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cascadia/TerminalSettingsModel/MTSMSettings.h | Added dragDropDelimiter to profile settings macro with default value L" " |
| src/cascadia/TerminalSettingsModel/Profile.idl | Added DragDropDelimiter as inheritable profile setting |
| src/cascadia/inc/ControlProperties.h | Added DragDropDelimiter to control properties macro |
| src/cascadia/TerminalControl/IControlSettings.idl | Exposed DragDropDelimiter getter in control settings interface |
| src/cascadia/TerminalControl/TermControl.cpp | Updated file drop logic to use configurable delimiter instead of hardcoded space |
| src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl | Added DragDropDelimiter to observable projected profile settings |
| src/cascadia/TerminalSettingsEditor/ProfileViewModel.h | Added macro entry for DragDropDelimiter observable setting |
| src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp | Added empty property change handler (unnecessary code added) |
| src/cascadia/TerminalSettingsEditor/Profiles_Advanced.xaml | Added TextBox UI control for drag/drop delimiter configuration |
| src/cascadia/TerminalSettingsAppAdapterLib/TerminalSettings.cpp | Initialized DragDropDelimiter from profile settings |
| src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw | Added localized strings with critical XML syntax error (missing closing tag) |
| doc/cascadia/profiles.schema.json | Added dragDropDelimiter schema entry with type string and default value |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </local:SettingContainer> | ||
|
|
||
| <!-- Drag Drop Delimiter --> | ||
| <local:SettingContainer x:Uid="Profile_DragDropDelimiter" |
There was a problem hiding this comment.
Missing x:Name attribute on the SettingContainer. All other SettingContainer elements in this file have x:Name attributes for consistency (e.g., "ShowMarks", "RainbowSuggestions", "PathTranslationStyle"). Add x:Name="DragDropDelimiter" to line 276.
| <local:SettingContainer x:Uid="Profile_DragDropDelimiter" | |
| <local:SettingContainer x:Name="DragDropDelimiter" | |
| x:Uid="Profile_DragDropDelimiter" |
There was a problem hiding this comment.
+1 to this change. We use x:Name to populate the search index.
src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
Outdated
Show resolved
Hide resolved
| else if (viewModelProperty == L"DragDropDelimiter") | ||
| { | ||
| // No specific computed property depends on this | ||
| // we can add _NotifyChanges(), if required | ||
| } |
There was a problem hiding this comment.
The empty else-if block is unnecessary. Settings that don't have derived properties don't need handlers in the PropertyChanged event. Consider removing this entire else-if block (lines 109-113) to reduce clutter. Many other simple settings like AllowVtClipboardWrite, RainbowSuggestions, etc. don't have handlers here.
| else if (viewModelProperty == L"DragDropDelimiter") | |
| { | |
| // No specific computed property depends on this | |
| // we can add _NotifyChanges(), if required | |
| } |
There was a problem hiding this comment.
+1 to this change. No need for an empty branch like that. Please remove it.
370f6b9 to
6e744cb
Compare
|
Hi, could someone review this PR? |
There was a problem hiding this comment.
Hey there! Sorry for the delay reviewing this. There's just a few minor changes I want to see before approving. Nice work and thanks for doing this 😊.
EDIT: Oh! Can you also update the documentation please? You'll want to submit a PR for this page here: https://github.com/MicrosoftDocs/terminal/blob/main/TerminalDocs/customize-settings/profile-advanced.md
| </local:SettingContainer> | ||
|
|
||
| <!-- Drag Drop Delimiter --> | ||
| <local:SettingContainer x:Uid="Profile_DragDropDelimiter" |
There was a problem hiding this comment.
+1 to this change. We use x:Name to populate the search index.
| else if (viewModelProperty == L"DragDropDelimiter") | ||
| { | ||
| // No specific computed property depends on this | ||
| // we can add _NotifyChanges(), if required | ||
| } |
There was a problem hiding this comment.
+1 to this change. No need for an empty branch like that. Please remove it.
| <value>Yes, clear the cache</value> | ||
| </data> | ||
| <data name="Profile_DragDropDelimiter.Header" xml:space="preserve"> | ||
| <value>Drag/Drop delimiter</value> |
There was a problem hiding this comment.
| <value>Drag/Drop delimiter</value> | |
| <value>Drag and drop delimiter</value> |
"Drag and drop" seems to be the main way this feature is referred to, so we should be consistent with that.
|
Extracted from PR body: Demodemo1.mp4 |
6e744cb to
a7596aa
Compare
|
Hi @carlos-zamora, thanks for reviewing! will open a separate PR to update the documentation for this feature as soon as I have some time to work on it. |
Summary of the Pull Request
This PR introduces a new profile setting,
dragDropDelimiter, which allows users to configure the string separator used when dragging and dropping multiple files into the terminal. The default behavior remains a single space (" ") for backward compatibility.References and Relevant Issues
Detailed Description of the Pull Request / Additional comments
Implementation Details:
DragDropDelimitertoMTSMSettings.handProfile.idl.ControlProperties.hso the terminal logic can see it.TermControl::OnDropto use the new delimiter when joining paths.Validation Steps Performed
dragDropDelimiterto", ",";", and custom strings insettings.jsonand from settings UI.Notes for Reviewers
Resources/en-USfile.ProfileViewModel.cpp, I added theelse ifblock for the new setting, but left it empty because no other UI logic currently depends on it.PR Checklist