Add auto-alignment of class definition with tabular

[Felixoid]

Hello.

I added automatical alignment of class definition with tabular plugin.

You could test it with next example:

class some::example (
    Type::Server::Hostname $hostname = $::hostname,
    Type::Inet $intern_ip = $::intern_ip,
    Optional[Type::Inet6] $primary_ip6 = defined('$::primary_ip6') ? {
        true => $::primary_ip6,
        false => undef,
    },
    Type::Server::Hostname $project = $::project,
    Enum[
        'online',
        'offline',
        'WIP',
    ] $state = $::state,
    String[1] $environment = $::environment,
    Optional[String[1]] $function = $::function,
    Optional[Type::Server::Hostname] $project_network = defined('$::project_network') ? {
        true => $::project_network,
        false => undef,
    },
    Type::Server::Hostname $monitoring_queue = $::monitoring_queue,
    Array[Pattern[/\A[a-z][a-z0-9_-]*\Z/]] $monitoring_checks = $::monitoring_checks,
) {
    file { '/just_example':
        ensure => 'directory',
    }
}

I'll be happy to help if you'll have any questions

[Felixoid]

Upd: the previous version has a bug with $attribute='value',, which fixed with the last commit.

Here updated test:

class some::example (
    Type::Server::Hostname $hostname = $::hostname,
    Type::Inet $intern_ip = $::intern_ip,
    Optional[Type::Inet6] $primary_ip6 = defined('$::primary_ip6') ? {
        true => $::primary_ip6,
        false => undef,
    },
    Type::Server::Hostname $project = $::project,
    Enum[
        'online',
        'offline',
        'WIP',
    ] $state= $::state,
    String[1] $environment =$::environment,
    Optional[String[1]] $function=$::function,
    Optional[Type::Server::Hostname] $project_network = defined('$::project_network') ? {
        true => $::project_network,
        false => undef,
    },
    Type::Server::Hostname $monitoring_queue = $::monitoring_queue,
    Array[Pattern[/\A[a-z][a-z0-9_-]*\Z/]] $monitoring_checks = $::monitoring_checks,
) {
    file { '/just_example':
        ensure => 'directory',
    }
}

[Felixoid]

I realized that it is possible to process the alignment of '=>' blocks in the same function. I'll try to do it nicely.

[Felixoid]

Right now everything works smoothly.

And again, class for tests:

class some::example (
    Type::Server::Hostname $hostname = $::hostname,
    Type::Inet $intern_ip = $::intern_ip,  
    Optional[Type::Inet6] $primary_ip6 = defined('$::primary_ip6') ? {
        true=>$::primary_ip6,
        false=>undef,
    },
    Type::Server::Hostname $project = $::project,
    Enum[
        'online',
        'offline',
        'WIP'
    ] $state= $::state,
    String[1] $environment =$::environment,
    Optional[String[1]] $function=$::function,
    Type::Server::Hostname $monitoring_queue = $::monitoring_queue,
    Array[Pattern[/\A[a-z][a-z0-9_-]*\Z/]] $monitoring_checks = $::monitoring_checks,
    Optional[Type::Server::Hostname] $project_network = defined('$::project_network') ? {
        true => $::project_network,
        false=> undef,
        default =>'',
    },
) {
    file { '/just_example':
        ensure => 'directory',
    }
}

[lelutin]

I've tested this both with the provided example and some other manifest I have and it's working nicely! it realigns the text when you press enter after having modified a line that needs realigning and aligns parameters after their type, too. Also it's fast at doing its job: if you end up realigning more than 10 lines there's no delay in doing so.

However it does not perform the realigning if you just open edition on a line (or just backspace to the end of the previous line while in edition mode) and press enter, and it also doesn't perform realignment if there are trailing spaces. I don't know if it's a limitation of the integration with tabular..

I do like this changeset though, it'll save me from having to call "Tabularize /=>" so many times

Here's an idea: could this tabular integration also be called when the '=' command is used to fix indentation?

[Felixoid]

Hi. Thank you for the feedback, @lelutin .
Your idea to map aligning on just <CR> is actually good! I'll add it.

About other things:

• Currently "aligning" mapped to ,, or to ,<CR>. That's why aligning doesn't apply when trailing spaces exist. Will be fixed on <CR> mapping.
• It's a bad idea to map action on just = because in that case, the filtering will not be applied properly (it could be applied to selectors accidentally)

[lelutin]

mapping to is really nice in terms of usability :)

also, thanks for the explanation about not being possible to apply filtering with the '=' operator.

I've just found a small bug though: with the following example:

class some_class (
  $arg1,
  $arg2,
  $arg3_string = 'Monitored',
  $arg4_boolean = false<cursor_here>
) {
  #...
}

if I open insert mode and press Enter at the end of any line that declares a parameter (see position of <cursor_here> in the above example, the equal signs get aligned correctly but the commas at the end of arg1 and arg2 lines also get moved to the right like this:

class some_class (
  $arg1         ,
  $arg2         ,
  $arg3_string   = 'Monitored',
  $arg4_boolean  = false
  <cursor_here>
) {
  #...
}

it would be nicer if the commas didn't get moved.

[Felixoid]

Thanks, nice catch

[lelutin]

nice thanks your last commit fixes moving commas.

I found another quirk though. with a slightly different example this time (now the arguments without an equal sign are longer than the ones with one):

class some_class (
  $arg1_long_name,
  $arg2_long_name_too,
  $arg3= 'Monitored',
  $arg4 = false<cursor_here>
) {
  #...
}

With the cursor at the end of a line of argument as shown above, if you press enter, the equal signs get moved all the way to the right of the longest argument without an equal sign, thus creating an awkward space like this:

class some_class (
  $arg1_long_name,
  $arg2_long_name_too,
  $arg3                = 'Monitored',
  $arg4                = false
  <cursor_here>
) {
  #...
}

[Felixoid]

But this is by design. We have three columns (type, argument, default value after =) and we align them. So, an aligning applied to the second column by first and then to third by second.

class some::example (
    $the_most_longest_argument,
    String $arg2,
    $arg3=4,
    Type::Server::Hostname $hostname =$::hostname,
    Type::Inet $intern_ip= $::intern_ip,
    Optional[Type::Inet6] $primary_ip6 = defined('$::primary_ip6') ? {
        true=>$::primary_ip6,
        false=> undef,
    },
    Type::Server::Hostname $project = $::project,
    Enum[
        'online',
        'offline',
        'WIP'
    ] $state= $::state,
    String[1] $environment = $::environment,
    Optional[String[1]] $function = $::function,
    Type::Server::Hostname $monitoring_queue = $::monitoring_queue,
    Array[Pattern[/\A[a-z][a-z0-9_-]*\Z/]] $monitoring_checks =$::monitoring_checks,
    Optional[Type::Server::Hostname] $project_network= defined('$::project_network') ? {
        true => $::project_network,
        false =>undef,
        default=>'',
    },
) {
    file { '/just_example':
        ensure => 'directory',
    }
}

You could try here on (at least at current moment =) ) the full example.
I understand what do you mean, but in case, when $arg and = are in fact delimiters (and this is the only way how we could identify columns) I didn't see the way how to ignore the length of mandatory arguments.

BTW, I found another one case, which wasn't covered, when class definition hadn't leading spaces.

[lelutin]

hmm there's another strangeness going on: in an empty file if I write this on the top level (not contained in anything):

$variable1 = 'some text'<press enter here>

the '=' gets moved uselessly by one space to the right:

$variable1  = 'some text'
<cursor is now here>

Also, if I remove the additional space, and then write something on the line just below and press enter:

$variable1 = 'some text'
some::defined_type { 'name':<press enter here>

then the '=' on the line for $variable1 gets moved to the right once again:

$variable1  = 'some text'
some::defined_type { 'name':
  <cursor now here>

[lelutin]

if I git checkout 6b49440 to remove the patch that had to do with parameters without a leading space then something similar is happening but instead of moving the '=' to the right, it's getting pushed left so that there's no space between $variable1 and the '=':

$variable1= 'some text'
<cursor is now here>

$variable1= 'some text'
some::defined_type { 'name':
  <cursor now here>

[Felixoid]

Thanks, also fixed. Realy nice catch!

[Felixoid]

And furthermore, this example looks ugly without last commit

class some::example (
$the_most_longest_argument,
String $arg2,
$arg3=4,
Type::Server::Hostname $hostname =$::hostname,
Type::Inet $intern_ip= $::intern_ip,
Optional[Type::Inet6] $primary_ip6 = defined('$::primary_ip6') ? {
    true=>$::primary_ip6,
    false=> undef,
},
# comment with => and $var inside
# comment with only =>
# comment with only $argument
Type::Server::Hostname $project = $::project,
Enum[
    'online',
    'offline',
    'WIP'
] $state= $::state,
String[1] $environment = $::environment,
Optional[String[1]] $function = $::function,
Type::Server::Hostname $monitoring_queue = $::monitoring_queue,
Array[Pattern[/\A[a-z][a-z0-9_-]*\Z/]] $monitoring_checks =$::monitoring_checks,
Optional[Type::Server::Hostname] $project_network= defined('$::project_network') ? {
    true => $::project_network,
    false =>undef,
    default=>'',
},
) {
    file { '/just_example':
        ensure => 'directory',
    }
}

[Felixoid]

Hello @rodjek
Could you leave a feedback, please? Will it be merged?

[Felixoid]

Found another bug. example:

# Another useful case
if $rsa_private_key != undef {
    some::cool::helper { $user:
        home_dir        => '/home/user',
        authorized_keys => [
            convert_ssh_rsa_private_to_public_key($rsa_private_key)
        ],
        rsa_private_key => $rsa_private_key,
    }
}

[lelutin]

hello! commit ceb00c6 breaks a bunch of use cases since it only does anything if the first line is defining a class.. e.g. what about defined types?

when there's one or more comment line(s) at the top of the file, which is common and recommended, then nothing happens. if there's one or more empty lines above the class definition then nothing happens.

hash definitions at the top level don't get realigned either (for example if I'm editing a "default.pp" file for use with vagrant, I'll be editing stuff at the top level, not inside a class definition)

[Felixoid]

@lelutin after some using of current configuration with inoremap <buffer> <silent> <CR> <Esc>:Tabularize puppet_class<CR>o I realized that it's too annoying when you try to insert exactly \n in manifest, because it doesn't. It completely changed UX for <Enter>.

I haven't idea right now how to do it properly. Tabular doesn't allow to use any of ms→`s for save+restore cursor because of strings changes. If you have an idea - I'll be glad to hear.

About last changes - yeah, there a lot of errors. As I said, I mentioned the bug on this example and tried to fix it.

Maybe, as a workaround, I could remove from first lines comments line by line until the ^\s*class will be there… Or switching off plain <Enter> and mapping on other keys will solve the problem also. That's a tricky part =\

Could you provide broken examples for tests?

[lelutin]

here are examples:

# this is a comment
class something (
  $blah,
  $argument1 = 'something',
  $arggg2 = 'something else',
)

class something (
  $blah,
  $argument1 = 'something',
  $arggg2 = 'something else',
)

$somehash = {
  'key1' => true,
  'another_key' => false,
} 

[lelutin]

hmm ctrl+a is by default used by vim for incrementing a number. the new mapping causes a clash there

[Felixoid]

heh, no =)

<C-a> is mapped as Tabularize only in insert mode, and it increases numbers in normal mode still.

[Felixoid]

Well, all examples work fine with <C-a>, and this part works fine with the classical => alignment.

[Felixoid]

@lelutin & @rodjek, could you merge it after some commits rebasing?

[Felixoid]

I added proper Vader tests during the last rebasing.