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.

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

Double and Triple Click Selection #1197

Merged
merged 17 commits into from
Jul 11, 2019

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Jun 10, 2019

Summary of the Pull Request

Initial double click selection works. So a double click will make a selection within some preset delimiters (' ', '/' '\'). The left side excludes the delimiter. The right side includes it.

Initial triple click selection works. Selects the entire line.

The following known issues will be implemented in separate PRs.
Known Issues:

  • ChunkSelection: move mouse after a double/triple click to select next chunk (section between delimiters)
  • Double Click Settings (Multi-Click Selection: Double-Click Settings #1273 ): configurable delimiters, enable chunk selection, set bounds as inclusive/exclusive

References

#1084 Screenshot Selection: one step closer to this. Now just need to create it and attach it as a configurable option to triple click
#988 Double Click Selection: needs ChunkSelection + Settings before completion
#989 Triple Click Selection: needs ChunkSelection + Settings before completion

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

TermControl Changes

  • Actually detect Double and Triple Click.
  • TIMESTAMP created to keep track of time delta for double/triple click (couldn't find a XAML way to do this :( )

Terminal Changes

  • Triple Click:
    • pretty straightforward. No delimiter check necessary.
  • Double Click:
    • if double click a delimiter, just select that spot
    • otherwise, expand selection to the left and right (each have their own function)
    • Delimiter check hardcodes space and slashes. This is super temporary because I'll replace it when delimiters are configurable in settings.

Validation Steps Performed

You can actually do most with the header on startup :).

  • Double Click first word in header
  • Double Click any other word in header
  • Triple Click any line

@carlos-zamora carlos-zamora self-assigned this Jun 10, 2019
src/cascadia/TerminalCore/Terminal.hpp Outdated Show resolved Hide resolved
src/cascadia/TerminalCore/Terminal.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalCore/Terminal.cpp Outdated Show resolved Hide resolved
Bugfix (double click after scrolling would go out of bounds)
@DHowett-MSFT DHowett-MSFT added the Area-User Interface 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
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.

There's one odd thing with the triple click check that I want to see your answer to before signing off.

src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalControl/TermControl.h Outdated Show resolved Hide resolved
src/cascadia/TerminalCore/Terminal.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalCore/Terminal.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalCore/Terminal.cpp Outdated Show resolved Hide resolved
@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jun 28, 2019
@carlos-zamora
Copy link
Member Author

Once #1273 gets merged in, I'm gonna do a bit of refactoring too. I want to move most/all of the changes in Terminal.cpp to TerminalSelection.cpp

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.

Approved, but please fix the seconds/milliseconds comment mismatch before merging.

src/cascadia/TerminalApp/Profile.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalControl/TermControl.h Outdated Show resolved Hide resolved
src/cascadia/TerminalCore/Terminal.hpp Outdated Show resolved Hide resolved
src/cascadia/TerminalCore/TerminalSelection.cpp Outdated Show resolved Hide resolved
@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 10, 2019
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 11, 2019
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 11, 2019
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

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

yes please

@carlos-zamora carlos-zamora merged commit 6d3001f into master Jul 11, 2019
@carlos-zamora carlos-zamora deleted the dev/cazamor/multi-click-selection branch July 11, 2019 23:06
mcpiroman pushed a commit to mcpiroman/terminal that referenced this pull request Jul 23, 2019
mcpiroman pushed a commit to mcpiroman/terminal that referenced this pull request Jul 23, 2019
@ghost
Copy link

ghost commented Aug 3, 2019

🎉Windows Terminal Preview v0.3.2142.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants