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 rc/auto-pairs #1295

Closed
wants to merge 1 commit into from
Closed

Add rc/auto-pairs #1295

wants to merge 1 commit into from

Conversation

alexherbo2
Copy link
Contributor

@alexherbo2 alexherbo2 commented Mar 24, 2017

Usage

hook global WinCreate .* %{
  auto-pairs-enable
}
map global user s :auto-pairs-surround<ret>

Status line integration

set global modelinefmt '… %opt(block_auto_pairs) …'

decl str block_auto_pairs

def -hidden block-update-auto-pairs %{ %sh{
  if [ $kak_opt_auto_pairs_surround_enabled = true ]; then
    text=surround
  else
    text="''"
  fi
  echo set window block_auto_pairs $text
}}

hook global WinCreate .* %{
  hook window InsertBegin .* block-update-auto-pairs
  hook window NormalBegin .* block-update-auto-pairs
}

Commands

  • auto-pairs-enable: enable automatic closing of pairs
  • auto-pairs-disable: disable automatic closing of pairs
  • auto-pairs-toggle: toggle automatic closing of pairs
  • auto-pairs-surround: enable automatic closing of pairs on selection boundaries for the whole insert session

Options

  • auto_pairs str-list: list of pairs (default: (,):{,}:[,]:<,>:",":',':<grave-quote>,<grave-quote>)
  • auto_pairs_enabled bool: information about the way auto-pairs is active (read-only)
  • auto_pairs_surround_enabled bool: information about the way auto-pairs-surround is active (read-only)

@srghma
Copy link

srghma commented Mar 24, 2017

@alexherbo2 , wow, it's like Christmas, thank you so much

@lenormf
Copy link
Contributor

lenormf commented Mar 24, 2017

I don't think this belongs in the upstream repository, it's better off as an extra script.

def -hidden auto-pairs-insert-backspace %{
try %{
%sh{
regex='\Q'$(echo "$kak_opt_auto_pairs" | sed s/:/'\\E|\\Q'/g';'s/,//g)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be wiser to use printf '%s\n' with arbitrary strings. Same remark for the following calls to echo.

%sh{
for pair in $(echo "$kak_opt_auto_pairs" | tr : '\n'); do
opener=$(echo $pair | cut --delimiter , --fields 1)
closer=$(echo $pair | cut --delimiter , --fields 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Long options are not defined by the standard, we better use -d and -f here. Same remark for the following calls.

@alexherbo2
Copy link
Contributor Author

I added some fixes that @casimir pointed out, notably his trick to preserve the selections

@alexherbo2
Copy link
Contributor Author

I changed the whole implementation to use hooks instead of maps

@alexherbo2
Copy link
Contributor Author

I added basic support for quotes

@danr
Copy link
Contributor

danr commented Mar 30, 2017

Surely there should be some tests for this feature and PR?

@casimir
Copy link
Contributor

casimir commented Mar 30, 2017

Long options for fold and tail are GNU-specific and don't work on macOS.

@alexherbo2
Copy link
Contributor Author

@casimir Does it work for you now?

@alexherbo2
Copy link
Contributor Author

Following our discussion on IRC, 😺 @mawww added InsertDelete hook, so we can get rid of <backspace> mapping now.

@casimir
Copy link
Contributor

casimir commented Mar 30, 2017

It works now but I get a weird selection when I insert a pair, one char to the right.

@alexherbo2
Copy link
Contributor Author

@danr Done :)

@srghma
Copy link

srghma commented Mar 30, 2017

I love it, I love it very much))
One more feature that seems for me indispensable is (from auto-pairs)

Insert new indented line after Return

input: {|} (press <CR> at |)
output: {
    |
}

@srghma
Copy link

srghma commented Mar 30, 2017

And I think Insert spaces before closing characters, only for [], (), {} would be nice to have too

@alexherbo2
Copy link
Contributor Author

@BjornMelgaard Done :)

@alexherbo2
Copy link
Contributor Author

Not polished yet though

@srghma
Copy link

srghma commented Apr 2, 2017

error running hook InsertChar( )/auto-pairs-insert: 1:1: 'auto-pairs-insert-space' 1:1: 'exec' no selections remaining
recursive call of hook InsertChar/
, not executing

Also waiting for implementation of idea that you told )

@alexherbo2
Copy link
Contributor Author

@BjornMelgaard I think the surround functionality could be superseded by the auto-pairs, using the selection set plus normal insertion

@srghma
Copy link

srghma commented Apr 10, 2017

@alexherbo2 yeah, I'm looking forward to it)

@alexherbo2
Copy link
Contributor Author

@BjornMelgaard I added the surround functionality

To use, craft your selections then execute :auto-pairs-surround command to enable automatic closing of pairs on selection boundaries for the whole insert session.

@srghma
Copy link

srghma commented May 29, 2017

@alexherbo2 finnaly, I will look at it right now )

@alexherbo2
Copy link
Contributor Author

@mawww I think the plug-in is ready to merge now.

Here is a recap on how to use it.

hook global WinCreate .* %{
  auto-pairs-enable
}
map global user s :auto-pairs-surround<ret>

@Delapouite
Copy link
Contributor

Delapouite commented Jun 1, 2017

This script is nice! Thanks to everyone involved.

May I suggest a few additions?

  • auto-pairs-toggle: to easily switch between auto-pairs-enable <-> auto-pairs-disable
  • auto-pairs-enabled readonly boolean option. This way in modelinefmt, one could display %opt{auto-pairs-enabled} as a visual signal that auto-pairs is currently active.
  • auto-pairs-surround-enabled same as above.

@Delapouite
Copy link
Contributor

Also a minor observation.

There are currently autowrap and autorestore scripts. Should this script be renamed autopairs (without the dash) to be consistent with previous commands?
This difference in the naming convention is mainly visible while using the fuzzy finder.

def -hidden auto-pairs-insert-new-line %{ try %{
%sh{
regex=$(printf '\Q%s\E' "$kak_opt_auto_pairs" | sed s/:/'\\E|\\Q'/g';'s/'<,>'/'<lt>,<gt>'/g';'s/,/'\\E\\n\\h*\\Q'/g)
echo "exec -draft %(;KGl<a-k>$regex<ret>)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the $regex variable contains backslashes, passing this string to echo is risky, better use printf.

@alexherbo2
Copy link
Contributor Author

@Delapouite I added the toggle command.

Commands:

  • auto-pairs-toggle

Options:

  • auto_pairs_enabled
  • auto_pairs_surround_enabled

And for the name, I prefer real word command / option names (#806).

@alexherbo2 alexherbo2 changed the title add rc/auto-pairs.kak Add rc/auto-pairs Jun 8, 2017
@alexherbo2
Copy link
Contributor Author

Some tests were failing due to not being POSIX.

I fixed them thanks to @lenormf.

@Delapouite
Copy link
Contributor

Threre seems to be an issue remaining on the auto_pairs_was_enabled bool.

Here's what I observed:

  • :auto-pairs-toggle
  • :auto-pairs-surround
  • <esc>
  • echo %opt{auto_pairs_enabled}
    -> false


def -hidden -params 2 auto-pairs-insert-opener %{ try %{
%sh{
if [ $1 = $2 ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs quotes around expansions.

def -hidden auto-pairs-delete-new-line %{ try %{
%sh{
regex=$(printf '\Q%s\E' "$kak_opt_auto_pairs" | sed s/:/'\\E|\\Q'/g';'s/'<,>'/'<lt>,<gt>'/g';'s/,/'\\E\\n\\h*\\Q'/g)
echo "exec -draft %(;JGi<a-k>$regex<ret>)"
Copy link
Contributor

Choose a reason for hiding this comment

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

printf is better here.

def -hidden auto-pairs-insert-space %{ try %{
%sh{
regex=$(printf '\Q%s\E' "$kak_opt_auto_pairs" | sed s/:/'\\E|\\Q'/g';'s/'<,>'/'<lt>,<gt>'/g';'s/,/'\\E\\h\\Q'/g)
echo "exec -draft %(;2H<a-k>$regex<ret>)"
Copy link
Contributor

Choose a reason for hiding this comment

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

printf is better here.

def -hidden auto-pairs-delete-space %{ try %{
%sh{
regex=$(printf '\Q%s\E' "$kak_opt_auto_pairs" | sed s/:/'\\E|\\Q'/g';'s/'<,>'/'<lt>,<gt>'/g';'s/,/'\\E\\h\\Q'/g)
echo "exec -draft %(;l2H<a-k>$regex<ret>)"
Copy link
Contributor

Choose a reason for hiding this comment

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

printf is better here.

def auto-pairs-enable -docstring 'Enable automatic closing of pairs' %{
%sh{
for pair in $(echo "$kak_opt_auto_pairs" | tr : '\n'); do
opener=$(echo $pair | cut -d , -f 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

printf is better here, also quotes around the expansion.

for pair in $(echo "$kak_opt_auto_pairs" | tr : '\n'); do
opener=$(echo $pair | cut -d , -f 1)
closer=$(echo $pair | cut -d , -f 2)
echo "hook window InsertChar \Q$opener -group auto-pairs-insert %(auto-pairs-insert-opener %-$opener- %-$closer-)"
Copy link
Contributor

Choose a reason for hiding this comment

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

printf is better here.


def auto-pairs-surround -docstring 'Enable automatic closing of pairs on selection boundaries for the whole insert session' %{
%sh{
if [ $kak_opt_auto_pairs_enabled = yes ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs quotes around expansions.

else
echo set window auto_pairs_was_enabled no
fi
for pair in $(echo "$kak_opt_auto_pairs" | tr : '\n'); do
Copy link
Contributor

Choose a reason for hiding this comment

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

printf is better here.

echo set window auto_pairs_was_enabled no
fi
for pair in $(echo "$kak_opt_auto_pairs" | tr : '\n'); do
opener=$(echo $pair | cut -d , -f 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

printf is better here, also quotes around the expansion.

fi
for pair in $(echo "$kak_opt_auto_pairs" | tr : '\n'); do
opener=$(echo $pair | cut -d , -f 1)
closer=$(echo $pair | cut -d , -f 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

printf is better here, also quotes around the expansion.

@alexherbo2
Copy link
Contributor Author

@Delapouite I can reproduce, but I did not isolate the bug yet.

@lenormf I added the POSIX changes you requested.

Copy link
Contributor

@Delapouite Delapouite left a comment

Choose a reason for hiding this comment

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

I think I found the issues:

  • a echo was missign from a %sh{} block
  • internally kakoune turn yes/no booleans into true/false booleans. So it has undesirable effects when comparing yes with a true.

So I would recommend to never use yes/no booleans, and always sticks with good ol' true/false

hook window InsertEnd .* -group auto-pairs-surround-insert-end %{
%sh{
if [ "$kak_opt_auto_pairs_was_enabled" = yes ]; then
auto-pairs-enable
Copy link
Contributor

Choose a reason for hiding this comment

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

missing echo

@@ -0,0 +1,131 @@
decl str-list auto_pairs %((,):{,}:[,]:<,>:",":',':`,`)
decl bool auto_pairs_enabled no
Copy link
Contributor

Choose a reason for hiding this comment

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

use false for this 3 booleans

hook window InsertDelete \n -group auto-pairs-delete auto-pairs-delete-new-line
hook window InsertChar \h -group auto-pairs-insert auto-pairs-insert-space
hook window InsertDelete \h -group auto-pairs-delete auto-pairs-delete-space
set window auto_pairs_enabled yes
Copy link
Contributor

Choose a reason for hiding this comment

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

use true

}

def auto-pairs-toggle -docstring 'Toggle automatic closing of pairs' %{ %sh{
if [ "$kak_opt_auto_pairs_enabled" = yes ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

use true


def auto-pairs-surround -docstring 'Enable automatic closing of pairs on selection boundaries for the whole insert session' %{
%sh{
if [ "$kak_opt_auto_pairs_enabled" = yes ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

use true

}
hook window InsertEnd .* -group auto-pairs-surround-insert-end %{
%sh{
if [ "$kak_opt_auto_pairs_was_enabled" = yes ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

use true

def auto-pairs-disable -docstring 'Disable automatic closing of pairs' %{
remove-hooks window auto-pairs-insert
remove-hooks window auto-pairs-delete
set window auto_pairs_enabled no
Copy link
Contributor

Choose a reason for hiding this comment

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

use false

remove-hooks window auto-pairs-surround-insert
remove-hooks window auto-pairs-surround-delete
remove-hooks window auto-pairs-surround-insert-end
set window auto_pairs_surround_enabled no
Copy link
Contributor

Choose a reason for hiding this comment

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

use false

set window auto_pairs_surround_enabled no
}
auto-pairs-disable
set window auto_pairs_surround_enabled yes
Copy link
Contributor

Choose a reason for hiding this comment

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

use true

if [ "$kak_opt_auto_pairs_enabled" = yes ]; then
echo set window auto_pairs_was_enabled yes
else
echo set window auto_pairs_was_enabled no
Copy link
Contributor

Choose a reason for hiding this comment

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

use false

@lenormf
Copy link
Contributor

lenormf commented Jun 16, 2017

This is a bit drastic, all that needs to be done is compare the shell boolean variables to true, the rest can still use yes/no/true/false.

@alexherbo2
Copy link
Contributor Author

@Delapouite Thanks!

@Delapouite
Copy link
Contributor

This is a bit drastic

Well, as in other languages like YAML or Coffeescript, which support similar booleans niceties, it turns out to be source of errors (plainly demonstrated here) for no real benefits in term of lisibility or human confort.

@alexherbo2
Copy link
Contributor Author

Boolean niceties are not a problem in general, but you’re right in the case of Kakoune, because it targets POSIX shell which has no boolean type.

@alexherbo2
Copy link
Contributor Author

@Delapouite Did you mind opening an issue on that?

@alexherbo2
Copy link
Contributor Author

@alexherbo2 alexherbo2 closed this Jun 24, 2017
@alexherbo2 alexherbo2 deleted the rc-auto-pairs branch June 24, 2017 14:30
@alexherbo2
Copy link
Contributor Author

#1333 (comment)

@lenormf
Copy link
Contributor

lenormf commented Jun 26, 2017

Reviving #561 eh?

@alexherbo2
Copy link
Contributor Author

I still not fond of decentralizing a whole repository, like kakoune-extra.

But cherry-picking an extension to have a life on his own.

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.

None yet

6 participants