Un-confuse __line type #65

New Issue

ungleich-gitea · 2021-11-20T13:23:24Z

ungleich-gitea commented

2021-11-20 13:23:24 +00:00

This issue is a follow-up to !831.

Personally, I think that the --regex option of the __line type is a bit confusing.
It is not clear to me (without reading the source) how it interacts with --before and --after.
Moreover, I'm wondering if the needle="line" mode is even required, as it should be possible to rewrite every line into a regular expression.

I'd propose to either deprecate --regex or merge --before and --after into --position before|after|end.

What do you think? Would you like me to go forward implementing such a change?

This issue is a follow-up to !831. Personally, I think that the `--regex` option of the `__line` type is a bit confusing. It is not clear to me (without reading the source) how it interacts with `--before` and `--after`. Moreover, I'm wondering if the `needle="line"` mode is even required, as it should be possible to rewrite every line into a regular expression. I'd propose to either deprecate `--regex` or merge `--before` and `--after` into `--position before|after|end`. What do you think? Would you like me to go forward implementing such a change?

ungleich-gitea commented

2021-11-20 15:33:33 +00:00

Could it be that !939 addresses the main issue regarding __line?

For simple things it has its place, but not having the functionality added in !939 had made __line mostly useless for me.

For bigger changes made from a single type I too use __file and __block is relegated for cases in which I may want a file to be modified by multiple types; this gets tricky if the concept of __block gets removed (aka: I think the 3 types have a use-case).

Could it be that !939 addresses the main issue regarding `__line`? For simple things it has its place, but not having the functionality added in !939 had made `__line` mostly useless for me. For bigger changes made from a single type I too use `__file` and `__block` is relegated for cases in which I may want a file to be modified by multiple types; this gets tricky if the concept of `__block` gets removed (aka: I think the 3 types have a use-case).

ungleich-gitea commented

2021-11-20 15:33:35 +00:00

__sshd is a good example where I wouldn't want to push the whole file, because it does have sane defaults.

this kind of stuff may be even useful for someone. not yet sure if i would use em.

I would certainly use __value_{ini,yaml,...} and would even dare writing the INI one in AWK, but YAML? hell no :-)

then again, not all software is written that in mind. often there are no sane defaults and you have to define everything.

My personal feeling is that if software does not have a sane default configuration, it's probably more broken on the inside. Which leads to the question: "do I really want to use such software? or can I find a better one?"
Seriously: who, if not the developers, should know which options are sane defaults?

then there is configuration file made by packager and you don't like it.

As a type writer, not really my business. It's the user who chose the "packager" (even if indirectly through their distro).
As a user I can always choose to completely rewrite the file if I'm unhappy with the maintainer's decisions.

or you can't load any extra config files.

The worst config formats are the ones which allow you to recursively include other files.
This makes processing, let alone modifying, them incredibly difficult.

and I'm not sure if this comment is ontopic anymore... 🤔

Maybe not ontopic to this issue, but I still think that this would be an important discussion to have…

`__sshd` is a good example where I wouldn't want to push the whole file, because it does have sane defaults. > this kind of stuff may be even useful for someone. not yet sure if i would use em. I would certainly use `__value_{ini,yaml,...}` and would even dare writing the INI one in AWK, but YAML? hell no :-) > then again, not all software is written that in mind. often there are no sane defaults and you have to define everything. My personal feeling is that if software does not have a sane default configuration, it's probably more broken on the inside. Which leads to the question: "do I really want to use such software? or can I find a better one?" Seriously: who, if not the developers, should know which options are sane defaults? > then there is configuration file made by packager and you don't like it. As a type writer, not really my business. It's the user who chose the "packager" (even if indirectly through their distro). As a user I can always choose to completely rewrite the file if I'm unhappy with the maintainer's decisions. > or you can't load any extra config files. The worst config formats are the ones which allow you to recursively include other files. This makes processing, let alone modifying, them incredibly difficult. > and I'm not sure if this comment is ontopic anymore... :thinking: Maybe not ontopic to this issue, but I still think that this would be an important discussion to have…

ungleich-gitea commented

2021-11-20 15:33:36 +00:00

sshd_config

I started to write __sshd year ago and then went with whole __file 😺

I've been thinking about writing a basic config parser for some formats

__file_yaml /path/to/file.yaml --name foo.bar.baz --value false

__file_ini /path/to/file.ini --section foo --name bar --value baz

this kind of stuff may be even useful for someone. not yet sure if i would use em.

What should a configuration management system do with existing configuration?

this is very philosophical question. I vote goes to: you only change upstream/packager config files when there's no other way. and if upstream breaks something, you go and help fixing it so it doesn't happen again. everybody wins.

then again, not all software is written that in mind. often there are no sane defaults and you have to define everything. then there is configuration file made by packager and you don't like it. or you can't load any extra config files.

it's a mess anyway out there.

so, I mostly use __file. there are some __key_values and __lines, but these are very carefully picked rare occasions.

and I'm not sure if this comment is ontopic anymore... 🤔

> sshd_config I started to write `__sshd` year ago and then went with whole `__file` :smiley_cat: > I've been thinking about writing a basic config parser for some formats `__file_yaml /path/to/file.yaml --name foo.bar.baz --value false` `__file_ini /path/to/file.ini --section foo --name bar --value baz` this kind of stuff may be even useful for someone. not yet sure if i would use em. > What should a configuration management system do with existing configuration? this is very philosophical question. I vote goes to: you only change upstream/packager config files when there's no other way. and if upstream breaks something, you go and help fixing it so it doesn't happen again. everybody wins. then again, not all software is written that in mind. often there are no sane defaults and you have to define everything. then there is configuration file made by packager and you don't like it. or you can't load any extra config files. it's a mess anyway out there. so, I mostly use `__file`. there are some `__key_value`s and `__line`s, but these are very carefully picked rare occasions. and I'm not sure if this comment is ontopic anymore... :thinking:

ungleich-gitea commented

2021-11-20 15:33:38 +00:00

You are raising multiple partially related issues here. I believe that whatever decision is taken what it runs down to is personal preference (or preference of the BDFL ;-))

every time i try to use __line i just give up and use __file and write entire file as I want 🤷

Hmm, this is a philosophical question (similar issue in !828 regarding __apt_conf type):
What should a configuration management system do with existing configuration?

One approach is to replace whatever config there is with your own (a scheme that seems to be popular in the Puppet ecosystem).
I, on the other hand, prefer config mgmt. to be as minimally invasive as possible, because oftentimes I just want to adjust certain settings which I care about.
Distros and other software usually (and hopefully :-)) provide good defaults which I would like to keep.

From a user's perspective it would be good if a common agreement for cdist types could be made on which approach should be taken.

also, showerthought: __block is basically like __line, only multiline.

Mostly, but there are differences.

__lines can be given a position in the file --before/--after, while __blocks are put at the bottom.
__blocks have a prefix and suffix, __lines do not.

Thinking about that: I do see use cases for __lines with a cdist marker (to recognise lines added by cdist) and sometimes I would like my __line to be wrapped with blank lines…

wild idea: can we merge these two?

While I am not opposed to this idea, I fear that the resulting type would be even more confusing than the two combined.

Another showerthought: Maybe the two types are just too generic for some/many use cases.
I've been thinking about writing a basic config parser for some formats (e.g. sshd_config) in AWK.
Or when Python types are here: Just get the file from the target, process it in Python and send it back 🤷

You are raising multiple partially related issues here. I believe that whatever decision is taken what it runs down to is personal preference (or preference of the BDFL ;-)) > every time i try to use `__line` i just give up and use `__file` and write entire file as I want :shrug: Hmm, this is a philosophical question (similar issue in !828 regarding `__apt_conf` type): What should a configuration management system do with existing configuration? One approach is to replace whatever config there is with your own (a scheme that seems to be popular in the Puppet ecosystem). I, on the other hand, prefer config mgmt. to be as minimally invasive as possible, because oftentimes I just want to adjust certain settings which I care about. Distros and other software usually (and hopefully :-)) provide good defaults which I would like to keep. From a user's perspective it would be good if a common agreement for cdist types could be made on which approach should be taken. > also, showerthought: `__block` is basically like `__line`, only multiline. Mostly, but there are differences. * `__line`s can be given a position in the file `--before`/`--after`, while `__block`s are put at the bottom. * `__blocks` have a prefix and suffix, `__line`s do not. Thinking about that: I do see use cases for `__line`s with a cdist marker (to recognise lines added by cdist) and sometimes I would like my `__line` to be wrapped with blank lines… > wild idea: can we merge these two? While I am not opposed to this idea, I fear that the resulting type would be even more confusing than the two combined. Another showerthought: Maybe the two types are just too generic for some/many use cases. I've been thinking about writing a basic config parser for some formats (e.g. sshd_config) in AWK. Or when Python types are here: Just get the file from the target, process it in Python and send it back :shrug:

ungleich-gitea commented

2021-11-20 15:33:39 +00:00

every time i try to use __line i just give up and use __file and write entire file as I want 🤷

also, showerthought: __block is basically like __line, only multiline.

wild idea: can we merge these two?

every time i try to use `__line` i just give up and use `__file` and write entire file as I want :shrug: also, showerthought: `__block` is basically like `__line`, only multiline. wild idea: can we merge these two?

allan_wind commented

2021-12-21 00:28:34 +00:00

I tried to use __line to comment out the value 'Browing On' and add a 'Browing Off' with:

__line /etc/cups/cupsd.conf:comment\
	--file /etc/cups/cupsd.conf\
	--line '# Browsing On'\
	--regex '^Browsing On'\
	--state replace

__line /etc/cups/cupsd.conf:disable\
	--after '^# Browsing On'\
	--file /etc/cups/cupsd.conf\
	--line 'Browsing Off'

/etc/cups/cupsd.conf:comment removes the line `Browsing On' and on each run append the line '# Browsing On' to EOL. The documentation says "If state is 'replace', ensure all lines matching the regular expression are exactly 'line'." which doen't seem to be the behavior observed.

/etc/cups/cupsd.conf:disable works as expected, appends Browing Off after the first instance of `# Browsing On'

I tried to use `__line` to comment out the value 'Browing On' and add a 'Browing Off' with: ``` __line /etc/cups/cupsd.conf:comment\ --file /etc/cups/cupsd.conf\ --line '# Browsing On'\ --regex '^Browsing On'\ --state replace __line /etc/cups/cupsd.conf:disable\ --after '^# Browsing On'\ --file /etc/cups/cupsd.conf\ --line 'Browsing Off' ``` `/etc/cups/cupsd.conf:comment` removes the line `Browsing On' and on each run append the line '# Browsing On' to EOL. The documentation says "If state is 'replace', ensure all lines matching the regular expression are exactly 'line'." which doen't seem to be the behavior observed. `/etc/cups/cupsd.conf:disable` works as expected, appends `Browing Off` after the first instance of `# Browsing On'

Sign in to join this conversation.