Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-12-15 Thread Zbigniew Jędrzejewski-Szmek
On Tue, Oct 21, 2014 at 10:55:46PM +0200, Ronny Chevalier wrote: +if (arg_transport != BUS_TRANSPORT_LOCAL) { +log_error(Cannot remotely edit units); +return -EINVAL; +} + +if (arg_runtime) { +

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-22 Thread Lennart Poettering
On Tue, 21.10.14 22:55, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: That way we wouldn't have to allocate an strv, and just open code the search logic. This would then mirror nicely how the pager logic works... I agree but I can't (or I don't know how to do this), because we need

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-22 Thread Lennart Poettering
On Tue, 21.10.14 23:04, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: I think we should really take some inspiration here from how git prepares the editor when asking for commit messages: adding a really useful commented help text to the end of the new file to edit would be really

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-22 Thread Christian Seiler
Am 2014-10-22 10:36, schrieb Lennart Poettering: We can do this for the pager because there is no arguments to give, here we have a list of paths. THinking about this: are all those editors actually fine with editing multiple files at once? is nano? I know for certain that nano, vim, emacs,

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-22 Thread Lennart Poettering
On Wed, 22.10.14 00:29, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: Nitpick: please no {} for single-line if blocks. Oh and about this one, since I saw you mentioning it a couple of times in other patches sent to the ML, maybe you should add this to the CODING_STYLE ? I don't see any

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-22 Thread Lennart Poettering
On Wed, 22.10.14 11:10, Christian Seiler (christ...@iwakd.de) wrote: Am 2014-10-22 10:36, schrieb Lennart Poettering: We can do this for the pager because there is no arguments to give, here we have a list of paths. THinking about this: are all those editors actually fine with editing

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-22 Thread Lennart Poettering
On Wed, 22.10.14 01:48, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote: On Sat, Oct 18, 2014 at 06:30:02PM +0200, Ronny Chevalier wrote: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/amendments.conf, or by copying the original unit

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-22 Thread Lennart Poettering
On Wed, 22.10.14 18:28, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: I'm not sure abou the name 'amendments.conf'. Wouldn't 'local.conf' be more idiomatic, and also easier to type? I was thinking about this too, and I wanted to propose override.conf instead? The word amendment

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Lennart Poettering
On Tue, 21.10.14 03:01, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: +static int get_editors(char ***editors) { +char **tmp_editors = strv_new(nano, vim, vi, NULL); Please avoid calling functions and declaring variables in one line. Also, there's an OOM check missing for

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Lennart Poettering
On Sat, 18.10.14 18:30, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: Three more suggestions after thinking about this a bit more... +static int unit_file_drop_in(const char *unit_name, const char *config_home, char **new_path) { +char *tmp_path; +int r; + +

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Ronny Chevalier
2014-10-21 11:01 GMT+02:00 Lennart Poettering lenn...@poettering.net: On Tue, 21.10.14 03:01, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: +static int get_editors(char ***editors) { +char **tmp_editors = strv_new(nano, vim, vi, NULL); Please avoid calling functions and

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Ronny Chevalier
2014-10-21 11:09 GMT+02:00 Lennart Poettering lenn...@poettering.net: On Sat, 18.10.14 18:30, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: Three more suggestions after thinking about this a bit more... +static int unit_file_drop_in(const char *unit_name, const char *config_home, char

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Ronny Chevalier
2014-10-21 0:51 GMT+02:00 Lennart Poettering lenn...@poettering.net: On Sat, 18.10.14 18:30, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: Looks pretty good. A few comments. + +static int unit_file_copy_if_needed(const char *unit_name, const char *fragment_path, char **new_path) { +

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Zbigniew Jędrzejewski-Szmek
On Wed, Oct 22, 2014 at 12:29:15AM +0200, Ronny Chevalier wrote: 2014-10-21 0:51 GMT+02:00 Lennart Poettering lenn...@poettering.net: On Sat, 18.10.14 18:30, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: Looks pretty good. A few comments. + +static int

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Oct 18, 2014 at 06:30:02PM +0200, Ronny Chevalier wrote: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/amendments.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. Then it

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Ronny Chevalier
2014-10-22 1:48 GMT+02:00 Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl: On Sat, Oct 18, 2014 at 06:30:02PM +0200, Ronny Chevalier wrote: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/amendments.conf, or by copying the original unit from

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Ronny Chevalier
2014-10-22 2:13 GMT+02:00 Ronny Chevalier chevalier.ro...@gmail.com: 2014-10-22 1:48 GMT+02:00 Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl: On Sat, Oct 18, 2014 at 06:30:02PM +0200, Ronny Chevalier wrote: It helps editing units by either creating a drop-in file, like

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-21 Thread Zbigniew Jędrzejewski-Szmek
On Wed, Oct 22, 2014 at 02:42:13AM +0200, Ronny Chevalier wrote: 2014-10-22 2:13 GMT+02:00 Ronny Chevalier chevalier.ro...@gmail.com: 2014-10-22 1:48 GMT+02:00 Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl: On Sat, Oct 18, 2014 at 06:30:02PM +0200, Ronny Chevalier wrote: It helps editing

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-20 Thread Ronny Chevalier
2014-10-21 0:51 GMT+02:00 Lennart Poettering lenn...@poettering.net: On Sat, 18.10.14 18:30, Ronny Chevalier (chevalier.ro...@gmail.com) wrote: Looks pretty good. A few comments. + +static int unit_file_copy_if_needed(const char *unit_name, const char *fragment_path, char **new_path) { +

[systemd-devel] [PATCH v3] systemctl: add edit verb

2014-10-18 Thread Ronny Chevalier
It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/amendments.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. Then it invokes an editor to the related files and daemon-reload is invoked