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

Wrap selected text with brackets/quotes #1452

Merged
merged 4 commits into from
Oct 15, 2022

Conversation

camc314
Copy link
Contributor

@camc314 camc314 commented Oct 7, 2022

when text is selected, and [, (, {, ", ' is typed, it wrapped the selected text with that character pair

Screen.Recording.2022-10-07.at.17.36.32.mov

@@ -210,7 +233,7 @@ impl Editor {
.map(|(idx, content)| {
let region = &selection.regions()[*idx];
(
Selection::region(region.start, region.end),
Selection::region(region.max(), region.max()),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure whether this will break something, but I feel like it makes a bit more sense...

edits after should be applied to the end of the selection right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This will be incorrect if the selection isn't a caret.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

forgot to ask, when will the selection not be a caret?

Copy link
Collaborator

Choose a reason for hiding this comment

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

when start != end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't workout what I'm missing here.

for edits_after we never really want to replace the text? I don't think.

Its only used for this + matching pairs (i.e. type "(" it automatically closes it) + I can't think of when this would be wrong

@codecov-commenter
Copy link

codecov-commenter commented Oct 7, 2022

Codecov Report

Merging #1452 (91935c2) into master (1d93d28) will increase coverage by 0.00%.
The diff coverage is 35.29%.

@@          Coverage Diff           @@
##           master   #1452   +/-   ##
======================================
  Coverage    6.63%   6.64%           
======================================
  Files         125     125           
  Lines       51326   51342   +16     
======================================
+ Hits         3406    3411    +5     
- Misses      47920   47931   +11     
Impacted Files Coverage Δ
lapce-core/src/editor.rs 29.37% <35.29%> (+0.02%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

// wrap the text with that char and its corresponding closing pair
if region.start != region.end
&& (matching_pair_type.is_some() || c == '"' || c == '\'')
&& ((matching_pair_type.unwrap_or(false))
Copy link
Collaborator

@dzhou121 dzhou121 Oct 10, 2022

Choose a reason for hiding this comment

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

You can replace

(matching_pair_type.is_some() || c == '"' || c == '\'')
                        && ((matching_pair_type.unwrap_or(false))
                            || c == '"'
                            || c == '\'')

with

matching_pair_type == Some(true) || c == '"' ||  == '\''

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice!

@camc314 camc314 force-pushed the cam/dev/wrap-selected-text branch from 42338a3 to 91935c2 Compare October 13, 2022 19:09
@dzhou121 dzhou121 merged commit 59b9a9d into lapce:master Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants