Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Multi-Click Selection: Double-Click Settings #1273

Merged
merged 5 commits into from
Jul 10, 2019
Merged

Multi-Click Selection: Double-Click Settings #1273

merged 5 commits into from
Jul 10, 2019

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Jun 14, 2019 •
edited
Loading

Summary of the Pull Request

Delimiters for double click can now be set in settings. This is on a per profile basis (shoutout to all the WSL users out there).

References

The first of a few additions to Multi-Click Selection (#1197).
Contributes to #988

PR Checklist

  • Doesn't close issue. Please read above.
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

  • TerminalSettings:
    • add property to winrt TerminalSettings
  • Terminal App:
    • read/write JSON for this new property
    • default set to " /\"
  • TerminalCore:
    • import/use delimiters from ICoreSettings

Validation Steps Performed

  • modified profiles.json. Then performed double click in an instance of that profile

@carlos-zamora carlos-zamora self-assigned this Jun 14, 2019
@carlos-zamora carlos-zamora mentioned this pull request Jun 17, 2019
Closed
5 tasks
@DHowett-MSFT DHowett-MSFT added the Area-UserInterface Issues pertaining to the user interface of the Console or Terminal label Jun 18, 2019
@DHowett-MSFT DHowett-MSFT mentioned this pull request Jun 22, 2019
Closed
@carlos-zamora carlos-zamora mentioned this pull request Jun 28, 2019
Merged
10 tasks
@carlos-zamora carlos-zamora force-pushed the dev/cazamor/mcs/2x-settings branch from 482d880 to c1a9a7e Compare June 28, 2019 16:45
miniksa
miniksa requested changes Jun 28, 2019
View reviewed changes
Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the one issue with holding onto a string_view in a persistent manner.

src/cascadia/TerminalCore/Terminal.hpp Outdated Show resolved Hide resolved
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 28, 2019
@ghost ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jul 5, 2019
@ghost
Copy link

ghost commented Jul 5, 2019

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@carlos-zamora carlos-zamora force-pushed the dev/cazamor/mcs/2x-settings branch from c1a9a7e to 69549d7 Compare July 5, 2019 20:01
@ghost ghost removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. labels Jul 5, 2019
zadjii-msft
zadjii-msft approved these changes Jul 8, 2019
View reviewed changes
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a suggestion, but I'm fine without changing it too

src/cascadia/TerminalApp/Profile.cpp Outdated Show resolved Hide resolved
miniksa
miniksa requested changes Jul 10, 2019
View reviewed changes
Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pending on what's going on with the default list, otherwise, I'm OK

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 10, 2019
@carlos-zamora
Changed to constant
70482d4 
Updated default delimiters
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 10, 2019
@carlos-zamora carlos-zamora merged commit 3e7f78b into dev/cazamor/multi-click-selection Jul 10, 2019
@carlos-zamora carlos-zamora deleted the dev/cazamor/mcs/2x-settings branch July 10, 2019 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlong11 dlong11 dlong11 left review comments

@miniksa miniksa miniksa requested changes

@zadjii-msft zadjii-msft zadjii-msft approved these changes

Assignees

@carlos-zamora carlos-zamora

Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@carlos-zamora @dlong11 @miniksa @zadjii-msft @DHowett-MSFT