Skip to content
This repository has been archived by the owner on May 25, 2019. It is now read-only.

Indenting expression needs to be improved #35

Open
blueyed opened this issue Apr 20, 2015 · 5 comments · May be fixed by #36
Open

Indenting expression needs to be improved #35

blueyed opened this issue Apr 20, 2015 · 5 comments · May be fixed by #36

Comments

@blueyed
Copy link

blueyed commented Apr 20, 2015 •
edited by chase

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 blueyed changed the title Indenting is wrong Indenting expression needs to be improved Apr 20, 2015
@blueyed
Copy link
Author

blueyed commented Apr 20, 2015

Questions:

  • what is ':\s*[>|]?$' supposed to match (given that it would be magic)?

@blueyed
Copy link
Author

blueyed commented Apr 20, 2015

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
Copy link
Contributor

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):

  • : matches a literal :
  • \s* matches 0 or more space or tab characters
  • [>|] matches a literal > or | character
  • ? matches a literal ?
  • $ matches the end of line

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 added a commit to blueyed/vim-ansible-yaml that referenced this issue Apr 20, 2015
@blueyed
WIP: indent: major rewrite
4e48950 
 - Use \v with all patterns.
 - Align list entries properly, using a new function.
 - Removes the ':\s*[>|]?$' case, which was not working anyway (depending on
   `magic`), but is unclear to me.
 - New settings: indent_list_entries to allow extra indent for list
   entries (on by default).

Fixes chase#35
@blueyed blueyed linked a pull request Apr 20, 2015 that will close this issue
Open
@blueyed
Copy link
Author

blueyed commented Apr 20, 2015

There is a PR now at: #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 (mrk21/yaml-vim#1)).

@AKurilin
Copy link

AKurilin commented Jun 2, 2015

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

WIP: indent: major rewrite blueyed/vim-ansible-yaml
3 participants
@blueyed @AKurilin @benjifisher