Re: [systemd-devel] [PATCH] systemctl: add edit verb
2014-11-28 23:41 GMT+01:00 Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl: On Fri, Nov 28, 2014 at 09:48:55PM +0100, Ronny Chevalier wrote: 2014-10-29 16:22 GMT+01:00 Ronny Chevalier chevalier.ro...@gmail.com: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- Is it ok if I apply this patch, or is there other remarks ? (Minus the recent log_error_errno changes) Yes please go ahead! Your patch was one of the big things on my TODO list, it's great that you that your account is finally created and you can push it yourself. I replied to the last version of the patch with some comments. Pushed. Thanks for the review Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
2014-10-29 16:22 GMT+01:00 Ronny Chevalier chevalier.ro...@gmail.com: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- Is it ok if I apply this patch, or is there other remarks ? (Minus the recent log_error_errno changes) lookup_paths_init does not concatenate root_dir, so I added a path_join with arg_root TODO | 4 +- man/less-variables.xml| 4 +- man/systemctl.xml | 64 +- src/systemctl/systemctl.c | 525 +- 4 files changed, 587 insertions(+), 10 deletions(-) diff --git a/TODO b/TODO index abe89b7..1cbedd4 100644 --- a/TODO +++ b/TODO @@ -84,7 +84,7 @@ Features: * systemctl: if it fails, show log output? -* maybe add systemctl edit that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them +* systemctl edit: add commented help text to the end, like git commit * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true... @@ -776,7 +776,7 @@ External: * zsh shell completion: - command verb -TAB should complete options, but currently does not - - systemctl add-wants,add-requires + - systemctl add-wants,add-requires, edit Regularly: diff --git a/man/less-variables.xml b/man/less-variables.xml index 09cbd42..0fb4d7f 100644 --- a/man/less-variables.xml +++ b/man/less-variables.xml @@ -6,7 +6,7 @@ titleEnvironment/title variablelist class='environment-variables' -varlistentry +varlistentry id='pager' termvarname$SYSTEMD_PAGER/varname/term listitemparaPager to use when @@ -17,7 +17,7 @@ option--no-pager/option./para/listitem /varlistentry -varlistentry +varlistentry id='less' termvarname$SYSTEMD_LESS/varname/term listitemparaOverride the default diff --git a/man/systemctl.xml b/man/systemctl.xml index 7cbaa6c..26f5235 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -465,7 +465,7 @@ along with systemd; If not, see http://www.gnu.org/licenses/. listitem paraWhen used with commandenable/command, - commanddisable/command, + commanddisable/command, commandedit/command, (and related commands), make changes only temporarily, so that they are lost on the next reboot. This will have the effect that changes are not made in subdirectories of @@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service filenamedefault.target/filename to the given unit./para /listitem /varlistentry + +varlistentry + termcommandedit replaceableNAME/replaceable.../command/term + + listitem +paraEdit a drop-in snippet or a whole replacement file if +option--full/option is specified, to extend or override the +specified unit./para + +paraDepending on whether option--system/option (the default), +option--user/option, or option--global/option is specified, +this create a drop-in file for each units either for the system, +for the calling user or for all futures logins of all users. Then +the editor (see section Environment below) is invoked on temporary +files which will be saved as their corresponding files if the editor +exited successfully./para + +paraIf option--full/option is specified, this will copy the +original units instead of creating drop-in files./para + +paraIf option--runtime/option is specified, the changes will +be made temporarily in filename/run/filename and they will be +lost on the next reboot./para + +paraIf the temporary file is empty the modification of the related +unit is canceled/para + +paraAfter the units have been edited, the systemd configuration is +reloaded (in a way that is equivalent to commanddaemon-reload/command), +but it does not restart or reload the units./para + +paraNote that this command cannot be used to
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Wed, Oct 29, 2014 at 04:22:02PM +0100, Ronny Chevalier wrote: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- lookup_paths_init does not concatenate root_dir, so I added a path_join with arg_root TODO | 4 +- man/less-variables.xml| 4 +- man/systemctl.xml | 64 +- src/systemctl/systemctl.c | 525 +- 4 files changed, 587 insertions(+), 10 deletions(-) diff --git a/TODO b/TODO index abe89b7..1cbedd4 100644 --- a/TODO +++ b/TODO @@ -84,7 +84,7 @@ Features: * systemctl: if it fails, show log output? -* maybe add systemctl edit that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them +* systemctl edit: add commented help text to the end, like git commit * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true... @@ -776,7 +776,7 @@ External: * zsh shell completion: - command verb -TAB should complete options, but currently does not - - systemctl add-wants,add-requires + - systemctl add-wants,add-requires, edit Regularly: diff --git a/man/less-variables.xml b/man/less-variables.xml index 09cbd42..0fb4d7f 100644 --- a/man/less-variables.xml +++ b/man/less-variables.xml @@ -6,7 +6,7 @@ titleEnvironment/title variablelist class='environment-variables' -varlistentry +varlistentry id='pager' termvarname$SYSTEMD_PAGER/varname/term listitemparaPager to use when @@ -17,7 +17,7 @@ option--no-pager/option./para/listitem /varlistentry -varlistentry +varlistentry id='less' termvarname$SYSTEMD_LESS/varname/term listitemparaOverride the default diff --git a/man/systemctl.xml b/man/systemctl.xml index 7cbaa6c..26f5235 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -465,7 +465,7 @@ along with systemd; If not, see http://www.gnu.org/licenses/. listitem paraWhen used with commandenable/command, - commanddisable/command, + commanddisable/command, commandedit/command, (and related commands), make changes only temporarily, so that they are lost on the next reboot. This will have the effect that changes are not made in subdirectories of @@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service filenamedefault.target/filename to the given unit./para /listitem /varlistentry + +varlistentry + termcommandedit replaceableNAME/replaceable.../command/term + + listitem +paraEdit a drop-in snippet or a whole replacement file if +option--full/option is specified, to extend or override the +specified unit./para + +paraDepending on whether option--system/option (the default), +option--user/option, or option--global/option is specified, +this create a drop-in file for each units either for the system, creates each unit +for the calling user or for all futures logins of all users. Then , +the editor (see section Environment below) is invoked on temporary see the Environment section below +files which will be saved as their corresponding files if the editor which will be written to the real location if the editor exits successfully. +exited successfully./para + +paraIf option--full/option is specified, this will copy the +original units instead of creating drop-in files./para + +paraIf option--runtime/option is specified, the changes will +be made temporarily in filename/run/filename and they will be +lost on the next reboot./para + +paraIf the temporary file is empty the modification of the related empty upon exit +unit is canceled/para . + +paraAfter the units have been edited, the systemd configuration is drop the 'the' in 'the systemd configuration' +reloaded (in a
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Fri, Nov 28, 2014 at 09:48:55PM +0100, Ronny Chevalier wrote: 2014-10-29 16:22 GMT+01:00 Ronny Chevalier chevalier.ro...@gmail.com: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- Is it ok if I apply this patch, or is there other remarks ? (Minus the recent log_error_errno changes) Yes please go ahead! Your patch was one of the big things on my TODO list, it's great that you that your account is finally created and you can push it yourself. I replied to the last version of the patch with some comments. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] systemctl: add edit verb
It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- changes: * --runtime is handled * changes are made atomically by creating temporary files * man page improved * no heap allocation for execlp editor * arg_root is handled properly TODO | 4 +- man/less-variables.xml| 4 +- man/systemctl.xml | 64 +- src/systemctl/systemctl.c | 525 +- 4 files changed, 587 insertions(+), 10 deletions(-) diff --git a/TODO b/TODO index abe89b7..1cbedd4 100644 --- a/TODO +++ b/TODO @@ -84,7 +84,7 @@ Features: * systemctl: if it fails, show log output? -* maybe add systemctl edit that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them +* systemctl edit: add commented help text to the end, like git commit * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true... @@ -776,7 +776,7 @@ External: * zsh shell completion: - command verb -TAB should complete options, but currently does not - - systemctl add-wants,add-requires + - systemctl add-wants,add-requires, edit Regularly: diff --git a/man/less-variables.xml b/man/less-variables.xml index 09cbd42..0fb4d7f 100644 --- a/man/less-variables.xml +++ b/man/less-variables.xml @@ -6,7 +6,7 @@ titleEnvironment/title variablelist class='environment-variables' -varlistentry +varlistentry id='pager' termvarname$SYSTEMD_PAGER/varname/term listitemparaPager to use when @@ -17,7 +17,7 @@ option--no-pager/option./para/listitem /varlistentry -varlistentry +varlistentry id='less' termvarname$SYSTEMD_LESS/varname/term listitemparaOverride the default diff --git a/man/systemctl.xml b/man/systemctl.xml index 7cbaa6c..26f5235 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -465,7 +465,7 @@ along with systemd; If not, see http://www.gnu.org/licenses/. listitem paraWhen used with commandenable/command, - commanddisable/command, + commanddisable/command, commandedit/command, (and related commands), make changes only temporarily, so that they are lost on the next reboot. This will have the effect that changes are not made in subdirectories of @@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service filenamedefault.target/filename to the given unit./para /listitem /varlistentry + +varlistentry + termcommandedit replaceableNAME/replaceable.../command/term + + listitem +paraEdit a drop-in snippet or a whole replacement file if +option--full/option is specified, to extend or override the +specified unit./para + +paraDepending on whether option--system/option (the default), +option--user/option, or option--global/option is specified, +this create a drop-in file for each units either for the system, +for the calling user or for all futures logins of all users. Then +the editor (see section Environment below) is invoked on temporary +files which will be saved as their corresponding files if the editor +exited successfully./para + +paraIf option--full/option is specified, this will copy the +original units instead of creating drop-in files./para + +paraIf option--runtime/option is specified, the changes will +be made temporarily in filename/run/filename and they will be +lost on the next reboot./para + +paraIf the temporary file is empty the modification of the related +unit is canceled/para + +paraAfter the units have been edited, the systemd configuration is +reloaded (in a way that is equivalent to commanddaemon-reload/command), +but it does not restart or reload the units./para + +paraNote that this command cannot be used to remotely edit units +and that you cannot temporarily edit units which are in +filename/etc/filename since they take precedence over +
[systemd-devel] [PATCH] systemctl: add edit verb
It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- lookup_paths_init does not concatenate root_dir, so I added a path_join with arg_root TODO | 4 +- man/less-variables.xml| 4 +- man/systemctl.xml | 64 +- src/systemctl/systemctl.c | 525 +- 4 files changed, 587 insertions(+), 10 deletions(-) diff --git a/TODO b/TODO index abe89b7..1cbedd4 100644 --- a/TODO +++ b/TODO @@ -84,7 +84,7 @@ Features: * systemctl: if it fails, show log output? -* maybe add systemctl edit that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them +* systemctl edit: add commented help text to the end, like git commit * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true... @@ -776,7 +776,7 @@ External: * zsh shell completion: - command verb -TAB should complete options, but currently does not - - systemctl add-wants,add-requires + - systemctl add-wants,add-requires, edit Regularly: diff --git a/man/less-variables.xml b/man/less-variables.xml index 09cbd42..0fb4d7f 100644 --- a/man/less-variables.xml +++ b/man/less-variables.xml @@ -6,7 +6,7 @@ titleEnvironment/title variablelist class='environment-variables' -varlistentry +varlistentry id='pager' termvarname$SYSTEMD_PAGER/varname/term listitemparaPager to use when @@ -17,7 +17,7 @@ option--no-pager/option./para/listitem /varlistentry -varlistentry +varlistentry id='less' termvarname$SYSTEMD_LESS/varname/term listitemparaOverride the default diff --git a/man/systemctl.xml b/man/systemctl.xml index 7cbaa6c..26f5235 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -465,7 +465,7 @@ along with systemd; If not, see http://www.gnu.org/licenses/. listitem paraWhen used with commandenable/command, - commanddisable/command, + commanddisable/command, commandedit/command, (and related commands), make changes only temporarily, so that they are lost on the next reboot. This will have the effect that changes are not made in subdirectories of @@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service filenamedefault.target/filename to the given unit./para /listitem /varlistentry + +varlistentry + termcommandedit replaceableNAME/replaceable.../command/term + + listitem +paraEdit a drop-in snippet or a whole replacement file if +option--full/option is specified, to extend or override the +specified unit./para + +paraDepending on whether option--system/option (the default), +option--user/option, or option--global/option is specified, +this create a drop-in file for each units either for the system, +for the calling user or for all futures logins of all users. Then +the editor (see section Environment below) is invoked on temporary +files which will be saved as their corresponding files if the editor +exited successfully./para + +paraIf option--full/option is specified, this will copy the +original units instead of creating drop-in files./para + +paraIf option--runtime/option is specified, the changes will +be made temporarily in filename/run/filename and they will be +lost on the next reboot./para + +paraIf the temporary file is empty the modification of the related +unit is canceled/para + +paraAfter the units have been edited, the systemd configuration is +reloaded (in a way that is equivalent to commanddaemon-reload/command), +but it does not restart or reload the units./para + +paraNote that this command cannot be used to remotely edit units +and that you cannot temporarily edit units which are in +filename/etc/filename since they take precedence over +filename/run/filename./para + /listitem +/varlistentry /variablelist /refsect2 @@
Re: [systemd-devel] [PATCH] systemctl: add edit verb
2014-10-13 13:40 GMT+02:00 David Herrmann dh.herrm...@gmail.com: Hi Hi, On Sat, Oct 11, 2014 at 8:17 PM, Daniel Buch boogiewasth...@gmail.com wrote: Nice, I was in the process of implementing this. Looks good to me. But I think it would be better to use vi instead of vim if no editor is set. Vim is not installed on every system as default but vi is most likely. I'd prefer doing nothing. vi has quite different behavior than vim, especially in Ex mode. And for people uncomfortable with 'vi' it's even worth. And everyone should have set EDITOR anyway.. Yes, the first version that I made just shown an error saying that you should set SYSTEMD_EDITOR or EDITOR, but the RFE suggested using vim, and since no one was against it, I thought this was common to use vi/vim when EDITOR is not set. And, even as a vim user, I agree that for many people vi/vim is really not a good choice. I will resend the patch with the modification. Thanks ! Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Fri, Oct 17, 2014 at 1:24 PM, Lennart Poettering lenn...@poettering.net wrote: Does this make sense? Speaking as a nano user and someone who barely knows how to quit vim, I still think the decision of the default editor should be vi or the distribution's choice. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Fri, 17.10.14 15:45, Mantas Mikulėnas (graw...@gmail.com) wrote: On Fri, Oct 17, 2014 at 3:11 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 17.10.14 14:29, Mantas Mikulėnas (graw...@gmail.com) wrote: Technically proficient people will set $EDITOR or $VISUAL anyway. Non-technical people won't. Non-technical people are likel to be totally lost in vi/vim. For those folks probably nano makes a better choice, simply because it shows an explanation of they supported keys at the bottom of the screen. Hence, I think doing a logic like this would make sense: 1) if $EDITOR is set, use it 2) otherwise: if $VISUAL is set, use it 3) otherwise: if nano exists, use it 4) otherwise: if vim exists, use it 5) otherwise: if vi exists, use it The list of editors seems fine. Normally $VISUAL would be first, followed by $EDITOR... (But in practice nobody sets them to different values anyway, since no programs aside from mailx care about the distinction. So it's fine either way, and just ignoring $VISUAL would be just as good.) Is there a real distinction between $VISUAL and $EDITOR? environ(7) makes no distinction, have any better docs that clarify the supposed distinction? It's much older than me, but as far as I know, one would have had $EDITOR set to 'ed' or 'ex' for dumb typewriter-based terminals (which were used before CRT terminals were a thing) and $VISUAL to 'vi' or 'emacs' for CRT-based terminals (which could show full-screen programs), and programs would start the right one depending on $TERM value. But these days, I haven't seen any program (other than mailx) actually care about the differences; they pretty much always go $VISUAL || $EDITOR || something || something || vi. So most people have both set to the same editor, just in case. So I'd safely get rid of $VISUAL I'd argue that if people really are strange enough to try to use such a dumb terminal and expect to get ed or ex, then they should just set $VISUAL/$EDITOR to that and be done. Or in other words: if that distinction really makes sense they should set the right editor themselves in their .profile, based on the $TERM they find set (or not set). Either way, I think we should honour $EDITOR because it makes sense, and $VISUAL because it's probably more universally used and for historical compatibility. I think Ronny's patch was pretty close to what I think it should work like. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Mon, 13.10.14 15:13, Simon McVittie (simon.mcvit...@collabora.co.uk) wrote: On 13/10/14 14:38, Dale R. Worley wrote: My general understanding is that the traditional behavior when you need an editor but the user hasn't specified one is to use vi, and so people who don't want vi *always* set $VISUAL in their environment. The Right Thing™ is distro-specific. Debian and its derivatives have sensible-editor(1) which is a shell script that uses $VISUAL, $EDITOR, nano[1] or vi; I would expect systemd in Debian to use sensible-editor as its fallback, either via a configure option or a patch. In distros without sensible-editor, I'm tempted to say the solution is stop being a distro without sensible-editor. New systemd API? :-) I am really not sure we should expose such an API from systemd. However, I do agree that the algorithm sensible-editor exposes is reasonable enough so that we should mimic it to a certain degree, and use as built-in logic. And I figure Ronny's code is already pretty close to that. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Mon, 13.10.14 13:40, David Herrmann (dh.herrm...@gmail.com) wrote: Hi On Sat, Oct 11, 2014 at 8:17 PM, Daniel Buch boogiewasth...@gmail.com wrote: Nice, I was in the process of implementing this. Looks good to me. But I think it would be better to use vi instead of vim if no editor is set. Vim is not installed on every system as default but vi is most likely. I'd prefer doing nothing. vi has quite different behavior than vim, especially in Ex mode. And for people uncomfortable with 'vi' it's even worth. And everyone should have set EDITOR anyway.. Technically proficient people will set $EDITOR or $VISUAL anyway. Non-technical people won't. Non-technical people are likel to be totally lost in vi/vim. For those folks probably nano makes a better choice, simply because it shows an explanation of they supported keys at the bottom of the screen. Hence, I think doing a logic like this would make sense: 1) if $EDITOR is set, use it 2) otherwise: if $VISUAL is set, use it 3) otherwise: if nano exists, use it 4) otherwise: if vim exists, use it 5) otherwise: if vi exists, use it Note that I myself feel a bit lost in nano, and am a vim user for cases like this, but I am still of the opinion that vi/vim is not what you want to throw non-unix guru in without a warning. Does this make sense? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Fri, Oct 17, 2014 at 2:24 PM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 13.10.14 13:40, David Herrmann (dh.herrm...@gmail.com) wrote: Hi On Sat, Oct 11, 2014 at 8:17 PM, Daniel Buch boogiewasth...@gmail.com wrote: Nice, I was in the process of implementing this. Looks good to me. But I think it would be better to use vi instead of vim if no editor is set. Vim is not installed on every system as default but vi is most likely. I'd prefer doing nothing. vi has quite different behavior than vim, especially in Ex mode. And for people uncomfortable with 'vi' it's even worth. And everyone should have set EDITOR anyway.. Technically proficient people will set $EDITOR or $VISUAL anyway. Non-technical people won't. Non-technical people are likel to be totally lost in vi/vim. For those folks probably nano makes a better choice, simply because it shows an explanation of they supported keys at the bottom of the screen. Hence, I think doing a logic like this would make sense: 1) if $EDITOR is set, use it 2) otherwise: if $VISUAL is set, use it 3) otherwise: if nano exists, use it 4) otherwise: if vim exists, use it 5) otherwise: if vi exists, use it The list of editors seems fine. Normally $VISUAL would be first, followed by $EDITOR... (But in practice nobody sets them to different values anyway, since no programs aside from mailx care about the distinction. So it's fine either way, and just ignoring $VISUAL would be just as good.) -- Mantas Mikulėnas graw...@gmail.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Fri, 17.10.14 14:29, Mantas Mikulėnas (graw...@gmail.com) wrote: Technically proficient people will set $EDITOR or $VISUAL anyway. Non-technical people won't. Non-technical people are likel to be totally lost in vi/vim. For those folks probably nano makes a better choice, simply because it shows an explanation of they supported keys at the bottom of the screen. Hence, I think doing a logic like this would make sense: 1) if $EDITOR is set, use it 2) otherwise: if $VISUAL is set, use it 3) otherwise: if nano exists, use it 4) otherwise: if vim exists, use it 5) otherwise: if vi exists, use it The list of editors seems fine. Normally $VISUAL would be first, followed by $EDITOR... (But in practice nobody sets them to different values anyway, since no programs aside from mailx care about the distinction. So it's fine either way, and just ignoring $VISUAL would be just as good.) Is there a real distinction between $VISUAL and $EDITOR? environ(7) makes no distinction, have any better docs that clarify the supposed distinction? To me it appears that $VISUAL is a bit more legacy thatn $EDITOR is... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Fri, 17.10.14 13:30, David Timothy Strauss (da...@davidstrauss.net) wrote: On Fri, Oct 17, 2014 at 1:24 PM, Lennart Poettering lenn...@poettering.net wrote: Does this make sense? Speaking as a nano user and someone who barely knows how to quit vim, I still think the decision of the default editor should be vi or the distribution's choice. Well, what's the distribution's choice? I mean, if distributions want to change things they can patch this in, they have that option anyway... Note that with the order i proposed, systems that have no nano installed will effectively only get vi/vim invoked here. I think nano is mostly installed only on systems where it also is the default, or where the user wants to use it, hence it's probably a good default, since there's reason enough to believe that if it is available it shall be used, and if it shall not be used it's probably not installed... In a way this mimics Debian's behaviour I figure, which also defaults to nano in sensible-editor, currently, AFAIK... That all said, this topic kinda enters bikeshed territory, so I figure we should just implement some alg, and stick with it. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Fri, Oct 17, 2014 at 3:11 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 17.10.14 14:29, Mantas Mikulėnas (graw...@gmail.com) wrote: Technically proficient people will set $EDITOR or $VISUAL anyway. Non-technical people won't. Non-technical people are likel to be totally lost in vi/vim. For those folks probably nano makes a better choice, simply because it shows an explanation of they supported keys at the bottom of the screen. Hence, I think doing a logic like this would make sense: 1) if $EDITOR is set, use it 2) otherwise: if $VISUAL is set, use it 3) otherwise: if nano exists, use it 4) otherwise: if vim exists, use it 5) otherwise: if vi exists, use it The list of editors seems fine. Normally $VISUAL would be first, followed by $EDITOR... (But in practice nobody sets them to different values anyway, since no programs aside from mailx care about the distinction. So it's fine either way, and just ignoring $VISUAL would be just as good.) Is there a real distinction between $VISUAL and $EDITOR? environ(7) makes no distinction, have any better docs that clarify the supposed distinction? It's much older than me, but as far as I know, one would have had $EDITOR set to 'ed' or 'ex' for dumb typewriter-based terminals (which were used before CRT terminals were a thing) and $VISUAL to 'vi' or 'emacs' for CRT-based terminals (which could show full-screen programs), and programs would start the right one depending on $TERM value. But these days, I haven't seen any program (other than mailx) actually care about the differences; they pretty much always go $VISUAL || $EDITOR || something || something || vi. So most people have both set to the same editor, just in case. So I'd safely get rid of $VISUAL -- Mantas Mikulėnas graw...@gmail.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
From: Mantas Mikulėnas graw...@gmail.com Normally $VISUAL would be first, followed by $EDITOR... (But in practice nobody sets them to different values anyway, since no programs aside from mailx care about the distinction. So it's fine either way, and just ignoring $VISUAL would be just as good.) I ran into an archived message which suggested what the traditional indicator was for visual vs. dumb terminals: If TERM is unset, or had the value dumb, then it's a dumb terminal. Otherwise, it's a visual terminal. Dale ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
My general understanding is equal to dale. But with some consideration it led me to agree with the warning approach. Its no big deal setting EDITOR. Den 14/10/2014 01.06 skrev Ronny Chevalier chevalier.ro...@gmail.com: 2014-10-13 16:13 GMT+02:00 Simon McVittie simon.mcvit...@collabora.co.uk : On 13/10/14 14:38, Dale R. Worley wrote: My general understanding is that the traditional behavior when you need an editor but the user hasn't specified one is to use vi, and so people who don't want vi *always* set $VISUAL in their environment. The Right Thing™ is distro-specific. Debian and its derivatives have sensible-editor(1) which is a shell script that uses $VISUAL, $EDITOR, nano[1] or vi; I would expect systemd in Debian to use sensible-editor as its fallback, either via a configure option or a patch. In distros without sensible-editor, I'm tempted to say the solution is stop being a distro without sensible-editor. New systemd API? :-) Before I resend the patch, we should agree on what to do here, for me it makes more sense to raise an error asking to set either EDITOR or SYSTEMD_EDITOR than running vi or a sensible-editor like you said. Because, someone used to its editor will quit vi or sensible-editor and will try to find out how to set the editor for systemctl (Even if it is well known for EDITOR or VISUAL, some people don't set these variables). If we raise an error, someone not used to these variables but having a favorite editor will know how to change this. And if this person does not have a favorite editor and never used one (unlikely but I think this is the purpose of the sensible-editor ?), we can advice to set EDITOR to nano or an equivalent. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
Hi On Sat, Oct 11, 2014 at 8:17 PM, Daniel Buch boogiewasth...@gmail.com wrote: Nice, I was in the process of implementing this. Looks good to me. But I think it would be better to use vi instead of vim if no editor is set. Vim is not installed on every system as default but vi is most likely. I'd prefer doing nothing. vi has quite different behavior than vim, especially in Ex mode. And for people uncomfortable with 'vi' it's even worth. And everyone should have set EDITOR anyway.. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
From: David Herrmann dh.herrm...@gmail.com On Sat, Oct 11, 2014 at 8:17 PM, Daniel Buch boogiewasth...@gmail.com wrote: Nice, I was in the process of implementing this. Looks good to me. But I think it would be better to use vi instead of vim if no editor is set. Vim is not installed on every system as default but vi is most likely. I'd prefer doing nothing. vi has quite different behavior than vim, especially in Ex mode. And for people uncomfortable with 'vi' it's even worth. And everyone should have set EDITOR anyway.. My general understanding is that the traditional behavior when you need an editor but the user hasn't specified one is to use vi, and so people who don't want vi *always* set $VISUAL in their environment. If you're being really nice, the program determines whether the terminal is dumb or visual, and then goes through the sequence of choices: 1) $VISUAL if the terminal is visual and it is specified 2) $EDITOR if it is specified (or if $VISUAL fails to execute) 3) vi if the terminal is visual 4) ed or ex (I think) (I don't know what the test for visual is. It may simply have been trying to execute $VISUAL (and vi) and seeing if they exit with an error.) Dale ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On 13/10/14 14:38, Dale R. Worley wrote: My general understanding is that the traditional behavior when you need an editor but the user hasn't specified one is to use vi, and so people who don't want vi *always* set $VISUAL in their environment. The Right Thing™ is distro-specific. Debian and its derivatives have sensible-editor(1) which is a shell script that uses $VISUAL, $EDITOR, nano[1] or vi; I would expect systemd in Debian to use sensible-editor as its fallback, either via a configure option or a patch. In distros without sensible-editor, I'm tempted to say the solution is stop being a distro without sensible-editor. New systemd API? :-) S [1] chosen for being small, self-documenting (hotkeys shown at the bottom of the screen), and unlike vi, not having a learning curve like a brick wall. I use vim myself, but I wouldn't want to inflict it on beginners. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
2014-10-13 16:13 GMT+02:00 Simon McVittie simon.mcvit...@collabora.co.uk: On 13/10/14 14:38, Dale R. Worley wrote: My general understanding is that the traditional behavior when you need an editor but the user hasn't specified one is to use vi, and so people who don't want vi *always* set $VISUAL in their environment. The Right Thing™ is distro-specific. Debian and its derivatives have sensible-editor(1) which is a shell script that uses $VISUAL, $EDITOR, nano[1] or vi; I would expect systemd in Debian to use sensible-editor as its fallback, either via a configure option or a patch. In distros without sensible-editor, I'm tempted to say the solution is stop being a distro without sensible-editor. New systemd API? :-) Before I resend the patch, we should agree on what to do here, for me it makes more sense to raise an error asking to set either EDITOR or SYSTEMD_EDITOR than running vi or a sensible-editor like you said. Because, someone used to its editor will quit vi or sensible-editor and will try to find out how to set the editor for systemctl (Even if it is well known for EDITOR or VISUAL, some people don't set these variables). If we raise an error, someone not used to these variables but having a favorite editor will know how to change this. And if this person does not have a favorite editor and never used one (unlikely but I think this is the purpose of the sensible-editor ?), we can advice to set EDITOR to nano or an equivalent. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] systemctl: add edit verb
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 the $SYSTEMD_EDITOR or $EDITOR or vim to the related files and daemon-reload is invoked when the editor exited successfully. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- TODO | 2 - man/journalctl.xml| 6 +- man/less-variables.xml| 40 +++--- man/localectl.xml | 6 +- man/loginctl.xml | 6 +- man/machinectl.xml| 6 +- man/systemctl.xml | 44 +- man/systemd-analyze.xml | 6 +- man/timedatectl.xml | 6 +- src/systemctl/systemctl.c | 346 +- 10 files changed, 435 insertions(+), 33 deletions(-) diff --git a/TODO b/TODO index 0aaa9f2..857bdd0 100644 --- a/TODO +++ b/TODO @@ -65,8 +65,6 @@ Features: * systemctl: if it fails, show log output? -* maybe add systemctl edit that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them - * dbus: add new message hdr field for allowing interactive auth, write spec for it. update dbus spec to mandate that unknown flags *must* be ignored... * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true... diff --git a/man/journalctl.xml b/man/journalctl.xml index 7fb6afc..d36889f 100644 --- a/man/journalctl.xml +++ b/man/journalctl.xml @@ -891,7 +891,11 @@ failure code is returned./para /refsect1 -xi:include href=less-variables.xml / +refsect1 +titleEnvironment/title + +xi:include href=less-variables.xml / +/refsect1 refsect1 titleExamples/title diff --git a/man/less-variables.xml b/man/less-variables.xml index 09cbd42..1b8aae0 100644 --- a/man/less-variables.xml +++ b/man/less-variables.xml @@ -2,28 +2,24 @@ !DOCTYPE book PUBLIC -//OASIS//DTD DocBook XML V4.5//EN http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd; -refsect1 -titleEnvironment/title +variablelist class='environment-variables' + varlistentry + termvarname$SYSTEMD_PAGER/varname/term -variablelist class='environment-variables' -varlistentry -termvarname$SYSTEMD_PAGER/varname/term + listitemparaPager to use when + option--no-pager/option is not given; + overrides varname$PAGER/varname. Setting + this to an empty string or the value + literalcat/literal is equivalent to passing + option--no-pager/option./para/listitem + /varlistentry -listitemparaPager to use when -option--no-pager/option is not given; -overrides varname$PAGER/varname. Setting -this to an empty string or the value -literalcat/literal is equivalent to passing -option--no-pager/option./para/listitem -/varlistentry + varlistentry + termvarname$SYSTEMD_LESS/varname/term -varlistentry -termvarname$SYSTEMD_LESS/varname/term - -listitemparaOverride the default -options passed to -commandless/command -(literalFRSXMK/literal)./para/listitem -/varlistentry -/variablelist -/refsect1 + listitemparaOverride the default + options passed to + commandless/command + (literalFRSXMK/literal)./para/listitem + /varlistentry +/variablelist diff --git a/man/localectl.xml b/man/localectl.xml index 38e73c7..7ae6c60 100644 --- a/man/localectl.xml +++ b/man/localectl.xml @@ -223,7 +223,11 @@ code otherwise./para /refsect1 -xi:include href=less-variables.xml / +refsect1 +titleEnvironment/title + +xi:include href=less-variables.xml / +/refsect1 refsect1 titleSee Also/title diff --git a/man/loginctl.xml b/man/loginctl.xml index 749db92..4754790 100644 --- a/man/loginctl.xml +++ b/man/loginctl.xml @@ -438,7 +438,11 @@ code otherwise./para /refsect1 -xi:include href=less-variables.xml / +refsect1 +titleEnvironment/title + +xi:include href=less-variables.xml / +/refsect1 refsect1 titleSee Also/title diff --git a/man/machinectl.xml b/man/machinectl.xml index 2f2e257..b95b7fe 100644 --- a/man/machinectl.xml +++ b/man/machinectl.xml @@ -281,7 +281,11 @@
Re: [systemd-devel] [PATCH] systemctl: add edit verb
Nice, I was in the process of implementing this. Looks good to me. But I think it would be better to use vi instead of vim if no editor is set. Vim is not installed on every system as default but vi is most likely. Den 11/10/2014 18.37 skrev Ronny Chevalier chevalier.ro...@gmail.com: 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 the $SYSTEMD_EDITOR or $EDITOR or vim to the related files and daemon-reload is invoked when the editor exited successfully. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- TODO | 2 - man/journalctl.xml| 6 +- man/less-variables.xml| 40 +++--- man/localectl.xml | 6 +- man/loginctl.xml | 6 +- man/machinectl.xml| 6 +- man/systemctl.xml | 44 +- man/systemd-analyze.xml | 6 +- man/timedatectl.xml | 6 +- src/systemctl/systemctl.c | 346 +- 10 files changed, 435 insertions(+), 33 deletions(-) diff --git a/TODO b/TODO index 0aaa9f2..857bdd0 100644 --- a/TODO +++ b/TODO @@ -65,8 +65,6 @@ Features: * systemctl: if it fails, show log output? -* maybe add systemctl edit that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them - * dbus: add new message hdr field for allowing interactive auth, write spec for it. update dbus spec to mandate that unknown flags *must* be ignored... * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true... diff --git a/man/journalctl.xml b/man/journalctl.xml index 7fb6afc..d36889f 100644 --- a/man/journalctl.xml +++ b/man/journalctl.xml @@ -891,7 +891,11 @@ failure code is returned./para /refsect1 -xi:include href=less-variables.xml / +refsect1 +titleEnvironment/title + +xi:include href=less-variables.xml / +/refsect1 refsect1 titleExamples/title diff --git a/man/less-variables.xml b/man/less-variables.xml index 09cbd42..1b8aae0 100644 --- a/man/less-variables.xml +++ b/man/less-variables.xml @@ -2,28 +2,24 @@ !DOCTYPE book PUBLIC -//OASIS//DTD DocBook XML V4.5//EN http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd -refsect1 -titleEnvironment/title +variablelist class='environment-variables' + varlistentry + termvarname$SYSTEMD_PAGER/varname/term -variablelist class='environment-variables' -varlistentry -termvarname$SYSTEMD_PAGER/varname/term + listitemparaPager to use when + option--no-pager/option is not given; + overrides varname$PAGER/varname. Setting + this to an empty string or the value + literalcat/literal is equivalent to passing + option--no-pager/option./para/listitem + /varlistentry -listitemparaPager to use when -option--no-pager/option is not given; -overrides varname$PAGER/varname. Setting -this to an empty string or the value -literalcat/literal is equivalent to passing -option--no-pager/option./para/listitem -/varlistentry + varlistentry + termvarname$SYSTEMD_LESS/varname/term -varlistentry -termvarname$SYSTEMD_LESS/varname/term - -listitemparaOverride the default -options passed to -commandless/command -(literalFRSXMK/literal)./para/listitem -/varlistentry -/variablelist -/refsect1 + listitemparaOverride the default + options passed to + commandless/command + (literalFRSXMK/literal)./para/listitem + /varlistentry +/variablelist diff --git a/man/localectl.xml b/man/localectl.xml index 38e73c7..7ae6c60 100644 --- a/man/localectl.xml +++ b/man/localectl.xml @@ -223,7 +223,11 @@ code otherwise./para /refsect1 -xi:include href=less-variables.xml / +refsect1 +titleEnvironment/title + +xi:include href=less-variables.xml / +/refsect1 refsect1 titleSee Also/title diff --git a/man/loginctl.xml b/man/loginctl.xml index 749db92..4754790 100644 --- a/man/loginctl.xml +++ b/man/loginctl.xml @@ -438,7 +438,11 @@ code otherwise./para /refsect1 -
