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

Unified sections and Vim folds. #156

Open
ypid opened this issue Jul 7, 2016 · 8 comments
Open

Unified sections and Vim folds. #156

ypid opened this issue Jul 7, 2016 · 8 comments

Comments

@ypid
Copy link
Member

ypid commented Jul 7, 2016

Edit: Approved format: https://github.com/ypid/yaml2rst/blob/feature/yaml-fold-strip/examples/fold-markers-debops.yml
Edit: Reasoning docs: https://github.com/ypid/yaml2rst/blob/feature/yaml-fold-strip/docs/fold-markers.rst

Proposal for new unified sections and Vim folds format:

Edit: Initial proposal: https://github.com/ypid/ypid-ansible-common/blob/47b0c3fbc3a59b78d3505016928fc209694cea0b/template_role/defaults/main.yml
(has been redesigned by @drybjed, see #156 (comment))

Refer to: debops/yaml2rst#3 and https://github.com/htgoebel/yaml2rst/blob/develop/docs/fold-markers.rst for details.

@drybjed
Copy link
Member

drybjed commented Jul 7, 2016

I suppose that ROLE_NAME__enabled variable has to go, to be consistent with other "state" variables. I don't mind that much. Actually, I though about making an alternative interface of sorts to enable/disable roles in one place using ansible_local.tags list, managed by debops.core role. Other roles could check that list for strings, but that probably adds some weird stuff (roles need to account for ansible_local.tags not being present in the state variable).

What worries me is that this change would imply that the role state is reversible, which is almost never true. I think that 90% of current roles don't allow for a reversible state change, either because they don't yet track everything a'la dpkg-divert, or they don't have tasks that would revert all of the changes. the ROLE_NAME__enabled variable was meant for roles included in the common.yml playbook, so that people who didn't want, say, debops.ferm messing with their firewall could just disable the role without the need to create custom playbook(s). Most of the roles are designed to "apply" configuration and don't bother with a way to revert that. The most common way to handle "reversion" would be to just recreate the container or reinstall the host - after all applying all of the changes again takes only minutes instead of hours. Therefore this variable style change would not be equivalent.

@ypid
Copy link
Member Author

ypid commented Jul 8, 2016

There was no ROLE_NAME__enabled, only a ROLE_NAME__deploy_state (which I now removed since this issue is not about that).

ypid added a commit to ypid/docs-1 that referenced this issue Jul 8, 2016
Related to: debops#156

Is currently based on development version of yaml2rst.
@drybjed
Copy link
Member

drybjed commented Jul 9, 2016

I would make a slight modification to the headers:

# Packages and installation [[[
# -----------------------------

This way, without the additions in yaml2rst and just by removing the #, Sphinx documenation will still compile correctly, and with yaml2rst modification removing the [[[ markers, headers will still work - in reStructuredText heading lines need to be at least as long as the text, but can be longer than it.

@drybjed
Copy link
Member

drybjed commented Jul 9, 2016

BTW, will the yaml2rst fold removal work with fold levels, like [[[1 ?

@ypid
Copy link
Member Author

ypid commented Jul 9, 2016

BTW, will the yaml2rst fold removal work with fold levels, like [[[1 ?

Sure. 😉 Covered by the regex: /\s*(:?\[{3}|\]{3})\d?$/ but I have found that explicitly specifying fold levels does not work so well for me (moving sections, changing levels).

This way, without the additions in yaml2rst and just by removing the #, Sphinx documenation will still compile correctly, and with yaml2rst modification removing the [[[ markers, headers will still work - in reStructuredText heading lines need to be at least as long as the text, but can be longer than it.

I see your point. Although we control the build software so I am not sure if we need to take care of the "without the additions in yaml2rst" case. But if you prefer this then lets go with that. As long as we can agree on something 😄

@ypid
Copy link
Member Author

ypid commented Jul 11, 2016

@drybjed Redesigned and improved the format in https://github.com/debops/ansible-unattended_upgrades/blob/master/defaults/main.yml. This format meets all the requirements and has been approved.

See: https://github.com/ypid/yaml2rst/blob/feature/yaml-fold-strip/docs/fold-markers.rst

@ypid ypid added the approved label Jul 11, 2016
@ypid
Copy link
Member Author

ypid commented Jul 11, 2016

One thing that @drybjed changed was to go from

# Main configuration [[[
# ------------------

to:

# Main configuration [[[
# ----------------------

This has the advantage that in case we ever manage to get syntax highlighting for the embedded RST syntax working in Vim, it will not complain about sections being to short. I tried it but I am not sure if that is possible as the # in the input for the syntax highlighting in Vim would need to be stipped out and then passed to the rst syntax highlighting. Ref: http://vim.wikia.com/wiki/Different_syntax_highlighting_within_regions_of_a_file#Extended_version

ypid added a commit to ypid/yaml2rst that referenced this issue Jul 12, 2016
Ref: debops/docs#156 (comment)

One thing that @drybjed changed was to go from

  ```YAML
  # Main configuration [[[
  # ------------------
  ```

to:

  ```YAML
  # Main configuration [[[
  # ----------------------
  ```

This has the advantage that in case we ever manage to get syntax highlighting for the embedded RST syntax working in Vim, it will not complain about sections being to short. I tried it but I am not sure if that is possible as the `# ` in the input for the syntax highlighting in Vim would need to be stipped out and then passed to the `rst` syntax highlighting. Ref: http://vim.wikia.com/wiki/Different_syntax_highlighting_within_regions_of_a_file#Extended_version
ypid added a commit to ypid/yaml2rst that referenced this issue Jul 12, 2016
ypid added a commit to ypid/yaml2rst that referenced this issue Jul 12, 2016
ypid added a commit to ypid/yaml2rst that referenced this issue Jul 12, 2016
ypid added a commit to ypid/yaml2rst that referenced this issue Jul 12, 2016
`--strip-regex` was intentionally limited to the RST part of the input
to avoid problems.
`--yaml-strip-regex` allows to mess with the YAML part of the input if
needed.

Related to: debops#3
Related to: debops/docs#156
ypid added a commit to ypid/docs-1 that referenced this issue Jul 12, 2016
@ypid
Copy link
Member Author

ypid commented Mar 14, 2017

Small inconvenience with the indented comments, yamllint complains about it in its default configuration:

yamllint ypid-ansible-common/roles/debops.cron/defaults/main.yml 
ypid-ansible-common/roles/debops.cron/defaults/main.yml
  19:16     warning  truthy value is not quoted  (truthy)
  21:68     warning  comment not indented like content  (comments-indentation)
  25:23     error    too many spaces inside brackets  (brackets)
  25:30     error    too many spaces inside brackets  (brackets)
  27:68     warning  comment not indented like content  (comments-indentation)
  32:68     warning  comment not indented like content  (comments-indentation)
  46:68     warning  comment not indented like content  (comments-indentation)
  53:68     warning  comment not indented like content  (comments-indentation)
  60:68     warning  comment not indented like content  (comments-indentation)
  67:68     warning  comment not indented like content  (comments-indentation)
  74:68     warning  comment not indented like content  (comments-indentation)
  82:68     warning  comment not indented like content  (comments-indentation)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants