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

Add option to not overtype on closing pairs #10501

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions book/src/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,18 +201,25 @@ Example:
Enables automatic insertion of pairs to parentheses, brackets, etc. Can be a
simple boolean value, or a specific mapping of pairs of single characters.

To disable auto-pairs altogether, set `auto-pairs` to `false`:
To disable auto-pairs altogether, set `config-type` to `false`:

```toml
[editor]
auto-pairs = false # defaults to `true`
[editor.auto-pairs]
config-type = false # defaults to `true`
Copy link
Contributor

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?

Copy link
Author

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.

Copy link
Member

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.

Copy link

@mathew-horner mathew-horner May 4, 2024

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.

```

To disable skipping over closing pairs, set `overtype` to `false`:

```toml
[editor.auto-pairs]
overtype = false #defaults to `true`
```

The default pairs are <code>(){}[]''""``</code>, but these can be customized by
setting `auto-pairs` to a TOML table:

```toml
[editor.auto-pairs]
[editor.auto-pairs.config-type]
'(' = ')'
'{' = '}'
'[' = ']'
Expand All @@ -231,7 +238,7 @@ Example `languages.toml` that adds `<>` and removes `''`
[[language]]
name = "rust"

[language.auto-pairs]
[language.auto-pairs.config-type]
'(' = ')'
'{' = '}'
'[' = ']'
Expand Down
14 changes: 10 additions & 4 deletions helix-core/src/auto_pairs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,13 @@ impl Default for AutoPairs {
// middle of triple quotes, and more exotic pairs like Jinja's {% %}

#[must_use]
pub fn hook(doc: &Rope, selection: &Selection, ch: char, pairs: &AutoPairs) -> Option<Transaction> {
pub fn hook(
doc: &Rope,
selection: &Selection,
ch: char,
pairs: &AutoPairs,
overtype: bool,
) -> Option<Transaction> {
log::trace!("autopairs hook selection: {:#?}", selection);

if let Some(pair) = pairs.get(ch) {
Expand All @@ -130,7 +136,7 @@ pub fn hook(doc: &Rope, selection: &Selection, ch: char, pairs: &AutoPairs) -> O
return Some(handle_open(doc, selection, pair));
} else if pair.close == ch {
// && char_at pos == close
return Some(handle_close(doc, selection, pair));
return Some(handle_close(doc, selection, pair, overtype));
}
}

Expand Down Expand Up @@ -301,7 +307,7 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
t
}

fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair, overtype: bool) -> Transaction {
let mut end_ranges = SmallVec::with_capacity(selection.len());
let mut offs = 0;

Expand All @@ -310,7 +316,7 @@ fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
let next_char = doc.get_char(cursor);
let mut len_inserted = 0;

let change = if next_char == Some(pair.close) {
let change = if next_char == Some(pair.close) && overtype {
// return transaction that moves past close
(cursor, cursor, None) // no-op
} else {
Expand Down
54 changes: 41 additions & 13 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pub fn deserialize_auto_pairs<'de, D>(deserializer: D) -> Result<Option<AutoPair
where
D: serde::Deserializer<'de>,
{
Ok(Option::<AutoPairConfig>::deserialize(deserializer)?.and_then(AutoPairConfig::into))
Ok(Option::<AutoPairConfigType>::deserialize(deserializer)?.and_then(AutoPairConfigType::into))
}

fn default_timeout() -> u64 {
Expand Down Expand Up @@ -560,46 +560,74 @@ pub enum IndentationHeuristic {
Hybrid,
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
pub struct AutoPairConfig {
pub config_type: AutoPairConfigType,

/// Whether to insert a closing bracket instead of automatically skipping over existing closing brackets.
pub overtype: bool,
}

impl AutoPairConfig {
pub fn get_pairs(&self) -> Option<AutoPairs> {
match &self.config_type {
AutoPairConfigType::Enable(false) => None,
AutoPairConfigType::Enable(true) => Some(AutoPairs::default()),
AutoPairConfigType::Pairs(pairs) => Some(AutoPairs::new(pairs.iter())),
}
}
}

impl Default for AutoPairConfig {
fn default() -> Self {
Self {
config_type: AutoPairConfigType::default(),
overtype: true,
}
}
}

/// Configuration for auto pairs
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields, untagged)]
pub enum AutoPairConfig {
pub enum AutoPairConfigType {
/// Enables or disables auto pairing. False means disabled. True means to use the default pairs.
Enable(bool),

/// The mappings of pairs.
Pairs(HashMap<char, char>),
}

impl Default for AutoPairConfig {
impl Default for AutoPairConfigType {
fn default() -> Self {
AutoPairConfig::Enable(true)
AutoPairConfigType::Enable(true)
}
}

impl From<&AutoPairConfig> for Option<AutoPairs> {
fn from(auto_pair_config: &AutoPairConfig) -> Self {
impl From<&AutoPairConfigType> for Option<AutoPairs> {
fn from(auto_pair_config: &AutoPairConfigType) -> Self {
match auto_pair_config {
AutoPairConfig::Enable(false) => None,
AutoPairConfig::Enable(true) => Some(AutoPairs::default()),
AutoPairConfig::Pairs(pairs) => Some(AutoPairs::new(pairs.iter())),
AutoPairConfigType::Enable(false) => None,
AutoPairConfigType::Enable(true) => Some(AutoPairs::default()),
AutoPairConfigType::Pairs(pairs) => Some(AutoPairs::new(pairs.iter())),
}
}
}

impl From<AutoPairConfig> for Option<AutoPairs> {
fn from(auto_pairs_config: AutoPairConfig) -> Self {
impl From<AutoPairConfigType> for Option<AutoPairs> {
fn from(auto_pairs_config: AutoPairConfigType) -> Self {
(&auto_pairs_config).into()
}
}

impl FromStr for AutoPairConfig {
impl FromStr for AutoPairConfigType {
type Err = std::str::ParseBoolError;

// only do bool parsing for runtime setting
fn from_str(s: &str) -> Result<Self, Self::Err> {
let enable: bool = s.parse()?;
Ok(AutoPairConfig::Enable(enable))
Ok(AutoPairConfigType::Enable(enable))
}
}

Expand Down
3 changes: 2 additions & 1 deletion helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3767,10 +3767,11 @@ pub mod insert {
let text = doc.text();
let selection = doc.selection(view.id);
let auto_pairs = doc.auto_pairs(cx.editor);
let overtype = cx.editor.config().auto_pairs.overtype;

let transaction = auto_pairs
.as_ref()
.and_then(|ap| auto_pairs::hook(text, selection, c, ap))
.and_then(|ap| auto_pairs::hook(text, selection, c, ap, overtype))
.or_else(|| insert(text, selection, c));

let (view, doc) = current!(cx.editor);
Expand Down
4 changes: 2 additions & 2 deletions helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,7 +1086,7 @@ impl Editor {
) -> Self {
let language_servers = helix_lsp::Registry::new(syn_loader.clone());
let conf = config.load();
let auto_pairs = (&conf.auto_pairs).into();
let auto_pairs = (&conf.auto_pairs).get_pairs();

// HAXX: offset the render area height by 1 to account for prompt/commandline
area.height -= 1;
Expand Down Expand Up @@ -1168,7 +1168,7 @@ impl Editor {
/// relevant members.
pub fn refresh_config(&mut self) {
let config = self.config();
self.auto_pairs = (&config.auto_pairs).into();
self.auto_pairs = (&config.auto_pairs).get_pairs();
self.reset_idle_timer();
self._refresh();
}
Expand Down