-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Added change_case command #441
Changes from 3 commits
1223606
48df076
e613403
d8aacc0
e0b0745
e3beccd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -186,6 +186,9 @@ impl Command { | |||||
extend_till_prev_char, | ||||||
extend_prev_char, | ||||||
replace, | ||||||
switch_case, | ||||||
switch_to_uppercase, | ||||||
switch_to_lowercase, | ||||||
page_up, | ||||||
page_down, | ||||||
half_page_up, | ||||||
|
@@ -780,6 +783,58 @@ fn replace(cx: &mut Context) { | |||||
}) | ||||||
} | ||||||
|
||||||
fn switch_case(cx: &mut Context) { | ||||||
let (view, doc) = current!(cx.editor); | ||||||
let transaction = | ||||||
Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { | ||||||
let text: Tendril = range | ||||||
.fragment(doc.text().slice(..)) | ||||||
.chars() | ||||||
.map(|ch| { | ||||||
if ch.is_lowercase() { | ||||||
ch.to_uppercase().collect() | ||||||
} else if ch.is_uppercase() { | ||||||
ch.to_lowercase().collect() | ||||||
} else { | ||||||
vec![ch] | ||||||
} | ||||||
}) | ||||||
.flatten() | ||||||
.collect(); | ||||||
|
||||||
(range.from(), range.to() + 1, Some(text)) | ||||||
}); | ||||||
|
||||||
doc.apply(&transaction, view.id); | ||||||
doc.append_changes_to_history(view.id); | ||||||
} | ||||||
|
||||||
fn switch_to_uppercase(cx: &mut Context) { | ||||||
let (view, doc) = current!(cx.editor); | ||||||
let transaction = | ||||||
Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { | ||||||
let text: Tendril = range.fragment(doc.text().slice(..)).to_lowercase().into(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're using to_lowercase in the uppercase method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops, copy paste error; apologies. |
||||||
|
||||||
(range.from(), range.to() + 1, Some(text)) | ||||||
}); | ||||||
|
||||||
doc.apply(&transaction, view.id); | ||||||
doc.append_changes_to_history(view.id); | ||||||
} | ||||||
|
||||||
fn switch_to_lowercase(cx: &mut Context) { | ||||||
let (view, doc) = current!(cx.editor); | ||||||
let transaction = | ||||||
Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { | ||||||
let text: Tendril = range.fragment(doc.text().slice(..)).to_lowercase().into(); | ||||||
|
||||||
(range.from(), range.to() + 1, Some(text)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think this should fix the bug. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this does fix it when having a selection of >1 character, it causes it not to work when having a selection of 1 character. While this is solvable; the replace command has the exact same issue. If I understand correctly, this behaviour is currently due to ranges being inclusive. archseer above suggested that this behaviour should be fixed later on in #376. If so, should I work around the bug, or accept it for now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's fine, you should accept it for now and we'll deal with it in #376 |
||||||
}); | ||||||
|
||||||
doc.apply(&transaction, view.id); | ||||||
doc.append_changes_to_history(view.id); | ||||||
} | ||||||
|
||||||
fn scroll(cx: &mut Context, offset: usize, direction: Direction) { | ||||||
use Direction::*; | ||||||
let (view, doc) = current!(cx.editor); | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, these methods return an iterator, hmm. Maybe we can use
flat_map
and directly process these?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, good point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So with
flat_map
you should be able to return these iterators directly without.collect()
. Might be a problem since all the branches return a different type though?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must admit; this isn't my area of expertise, but I don't see much of an alternative. Or at-least no ones which also allocate.
With the exception of a custom enum combining the three branches, which also implements Iterator.
Any other suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the current implementation is fine for now :)