chase / vim-ansible-yaml

Add additional support for Ansible in VIM
449 stars 77 forks source link

Indenting expression needs to be improved #35

Open blueyed opened 9 years ago

blueyed commented 9 years ago

Given the playbook from http://docs.ansible.com/playbooks_intro.html:


---
- hosts: webservers
  vars:
    http_port: 80
    max_clients: 200
  remote_user: root
  tasks:
  - name: ensure apache is at the latest version
    yum: pkg=httpd state=latest
  - name: write the apache config file
    template: src=/srv/httpd.j2 dest=/etc/httpd.conf
    notify:
    - restart apache
  - name: ensure apache is running (and enable it at boot)
    service: name=httpd state=started enabled=yes
  handlers:
    - name: restart apache
      service: name=httpd state=restarted

gg=G results in:


---
- hosts: webservers
  vars:
  http_port: 80
  max_clients: 200
  remote_user: root
  tasks:
  - name: ensure apache is at the latest version
    yum: pkg=httpd state=latest
    - name: write the apache config file
      template: src=/srv/httpd.j2 dest=/etc/httpd.conf
      notify:
      - restart apache
      - name: ensure apache is running (and enable it at boot)
        service: name=httpd state=started enabled=yes
        handlers:
        - name: restart apache
          service: name=httpd state=restarted

The indenting code seems to mix expressions that rely on different magic settings for Vim: https://github.com/chase/vim-ansible-yaml/blob/master/indent/ansible.vim#L40-51

E.g. ':\s*[>|]?$' should be '\v:\s*[>|]?$' probably, and all patterns should explicitly state what mode to use, e.g. via \v.

But that is not enough to fix this.

I've tried to address this, and the result would be:


---
- hosts: webservers
  vars:
    http_port: 80
    max_clients: 200
  remote_user: root
  tasks:
  - name: ensure apache is at the latest version
    yum: pkg=httpd state=latest
  - name: write the apache config file
    template: src=/srv/httpd.j2 dest=/etc/httpd.conf
    notify:
    - restart apache
  - name: ensure apache is running (and enable it at boot)
    service: name=httpd state=started enabled=yes
  handlers:
  - name: restart apache
    service: name=httpd state=restarted

This changes (decreases) the indentation of the handlers list, which is in line with the other lists.

Since I am new to Ansible I might be missing something, and probably there are different styles of indenting list entries?

However, keeping them in line to the start of the list made it easier to handle. If I remember correctly it would not be possible to handle gg=G then in all cases, because the previous lines would have been indented already.

I will provide a PR for this.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/11556485-indenting-expression-needs-to-be-improved?utm_campaign=plugin&utm_content=tracker%2F509109&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F509109&utm_medium=issues&utm_source=github).
blueyed commented 9 years ago

Questions:

blueyed commented 9 years ago

For reference, the output from http://www.yamllint.com/ is the following:

--- 
- 
  handlers: 
    - 
      name: "restart apache"
      service: "name=httpd state=restarted"
  hosts: webservers
  remote_user: root
  tasks: 
    - 
      name: "ensure apache is at the latest version"
      yum: "pkg=httpd state=latest"
    - 
      name: "write the apache config file"
      notify: 
        - "restart apache"
      template: "src=/srv/httpd.j2 dest=/etc/httpd.conf"
    - 
      name: "ensure apache is running (and enable it at boot)"
      service: "name=httpd state=started enabled=yes"
  vars: 
    http_port: 80
    max_clients: 200
benjifisher commented 9 years ago

I am aware that there is room for improvement in the indentation code. The only promise I will make about when I might do something about it is that it will be sooner if you suggest a patch than if you do not.

As for the pattern ':\s*[>|]?$', assuming the default setting (magic):

I think you are right: the ? was probably intended as a 0-or-1 quantifier for the [>|]. My preference would be to make it \?, but you could also make the whole pattern very magic.

blueyed commented 9 years ago

There is a PR now at: https://github.com/chase/vim-ansible-yaml/pull/36 (likely to cause regressions).

I've also contacted the maintainer of the indenter in Vim's runtime (@ZyX-I), maybe there is an update since 2012.

And last but not least I've found: https://github.com/mrk21/yaml-vim/ (which also does not work well with Ansible's example (https://github.com/mrk21/yaml-vim/issues/1)).

akurilin commented 9 years ago

I've been experiencing a similar issue when trying to indent yaml lists with this plugin: