-
-
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
Add option to not overtype on closing pairs #10501
base: master
Are you sure you want to change the base?
Conversation
This is an extension of #10301. I apologize for the mistake in closing the request, as I am still learning how to use git! |
[editor] | ||
auto-pairs = false # defaults to `true` | ||
[editor.auto-pairs] | ||
config-type = false # defaults to `true` |
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.
Not really nice to break a config without much of a need. Perhaps it's better to add auto-pairs-overtype = <bool>
param?
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.
Do you mean adding an auto-pairs-overtype = <bool>
param to the Editor
? If so I can certainly do that. It seemed better to group it with the other auto-pairs
configuration.
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.
Yeah I suggested this as an option but whether a breaking change like this would be accepted would be up to the maintainers. It feels better to group them together since we do that for e.g. soft-wrap and the file picker. It would be possible to avoid a breaking change by using an enum, but yeah it's up to the maintainers.
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.
The config-type
naming is not very understandable to me, personally.
Also I think having both [editor.auto-pairs]
and [editor.auto-pairs.<whatever_we_name_it>]
complicates configuration. It would be nice if overtype
was co-located with the pair definitions. This would also maintain backwards compatability, if you make it optional.
helix-core/src/auto_pairs.rs
Outdated
log::trace!("autopairs hook selection: {:#?}", selection); | ||
|
||
if let Some(pair) = pairs.get(ch) { | ||
if pair.same() { | ||
return Some(handle_same(doc, selection, pair)); | ||
} else if pair.open == ch { | ||
return Some(handle_open(doc, selection, pair)); | ||
} else if pair.close == ch { | ||
} else if pair.close == ch && overtype { |
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.
Shouldn't the flag be handled inside handle_close
?
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 thought it might make more sense to just return and insert the closing pair instead of creating a new transaction and then checking. It definitely makes more sense inside though.
Solution to #9595
Vscode uses three modes(always, auto - only skip over auto generated closing pairs, and never) for autoCloseOvertype, while my implementation only uses two(always and never).