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

Auto-pairs for multiple characters #4035

Open
the-mikedavis opened this issue Sep 29, 2022 · 9 comments
Open

Auto-pairs for multiple characters #4035

the-mikedavis opened this issue Sep 29, 2022 · 9 comments
Labels
A-core Area: Helix core improvements C-enhancement Category: Improvements

Comments

@the-mikedavis
Copy link
Member

Auto-pairs currently only work for single characters, like pairing ( and ). Multi-character auto-pairs are also useful in some languages. For example, ``` pairs in markdown.

@the-mikedavis the-mikedavis added C-enhancement Category: Improvements A-core Area: Helix core improvements labels Sep 29, 2022
@poliorcetics
Copy link
Contributor

``` is possibly the worst example of this because it interferes greatly with the single ` and as such is very hard to do.

There is one more complex, which is auto-closing HTML/XML-like tags, like <code>| -> <code>|</code>, because it's dynamic, but this one is often done by LSPs, so Helix doesn't need to know about it.

@bindsdev
Copy link

bindsdev commented Jan 14, 2023

I'd be happy to help implement this. I have started some work but before I continue I wanted to discuss a few things. There was a couple approaches I was looking at for changes to the Pair struct:

  • Change Pair to an enum so it can contain a variant for both a single and a multi character pairing:
#[derive(Debug, Clone, Copy)]
pub enum Pair {
	Single {
		open: char,
		close: char,
	},
	Multi {
		open: &'static str,
		close: &'static str,
	},
}

This could also apply if a separate struct for a single and multi character pairing was made, but I believe that is unnecessary as we can just have struct-like enum variants and an enum would probably have to be used anyways to we could keep the HashMap<T, Pair>.

  • Keep the current Pair struct, but change the type of open and close to &'static str:
#[derive(Debug, Clone, Copy)]
pub struct Pair {
	pub open: &'static str,
	pub close: &'static str,
}

This would mean we wouldn't have to have to have an enum. In addition, &str would obviously be more flexible since it would allow for both single and multiple character opening and closing delimiters. In addition, the key type in the HashMap could be changed to &'static str.

Let me know your thoughts and suggestions!

@bindsdev
Copy link

I'd be happy to help implement this. I have started some work but before I continue I wanted to discuss a few things. There was a couple approaches I was looking at for changes to the Pair struct:

  • Change Pair to an enum so it can contain a variant for both a single and a multi character pairing:
#[derive(Debug, Clone, Copy)]
pub enum Pair {
	Single {
		open: char,
		close: char,
	},
	Multi {
		open: &'static str,
		close: &'static str,
	},
}

This could also apply if a separate struct for a single and multi character pairing was made, but I believe that is unnecessary as we can just have struct-like enum variants and an enum would probably have to be used anyways to we could keep the HashMap<T, Pair>.

  • Keep the current Pair struct, but change the type of open and close to &'static str:
#[derive(Debug, Clone, Copy)]
pub struct Pair {
	pub open: &'static str,
	pub close: &'static str,
}

This would mean we wouldn't have to have to have an enum. In addition, &str would obviously be more flexible since it would allow for both single and multiple character opening and closing delimiters. In addition, the key type in the HashMap could be changed to &'static str.

Let me know your thoughts and suggestions!

Any word, ideas, thoughts, or suggestions on this?

@kneasle
Copy link
Contributor

kneasle commented Jan 30, 2023

I haven't done any work on Helix's code before, but it seems to be that the second option is by far the cleanest - the Single variant is clearly a special case of Multi, so we may was well handle them in the same code path.

If I were you, I'd just give it a go and see what happens. Worst case, it doesn't work and you come away with more knowledge :)

@mangas
Copy link
Contributor

mangas commented Sep 15, 2023

@bindsdev did you end up making any progress on this issue? Are you still working on it?

@eliasp
Copy link

eliasp commented Sep 15, 2023

I stumbled upon this issue, as I was just trying to configure the auto-pair '{' = '};' for Nix, where in my use-case, 99% of the time this is what I need and a plain '{' = '}' is rather the exception.

@jtrees
Copy link

jtrees commented Sep 18, 2023

I stumbled upon this issue, as I was just trying to configure the auto-pair '{' = '};' for Nix, where in my use-case, 99% of the time this is what I need and a plain '{' = '}' is rather the exception.

Sorry for going slightly off-topic, but I can recommend using '=' and ';' as pairs in nix. I set that up as a workaround a few weeks ago and now it feels so natural to me I think I'll keep it that way even once multi-character pairs are possible.

@lemontheme
Copy link

Found this thread after noticing quote pairs don't work with Python's f-strings. The chain of linked issues led me here.

If the following config were valid, f-strings should work with quote auto-pairs.

[[language]]
name = "python"

[language.auto-pairs]
'f"' = '"'

Well, maybe at least. For instance, I'm not sure what would happen in this example:

"this is commander worf""
                        ^unintended extra " (y/n?)

This is the sort of thing where I'd be tempted to reach for a regular expression, e.g. /\bf\"/ = '"', but perhaps that's overkill.

Anyway, those are just my thoughts as a user. Up till now I've had to do next to no work to get Helix to work well with Python. Really enjoying it. <3 Just a bit surprised this didn't work out of the box, since f-strings are by far the more 'blessed' of Python's three mechanisms for string formatting.

@bindsdev
Copy link

@bindsdev did you end up making any progress on this issue? Are you still working on it?

Sorry, I am just seeing this. No, I am not working on it anymore. Everything got busy. You are welcome to take it on if you'd like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Helix core improvements C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

8 participants