Re: [systemd-devel] [PATCH 3/4] systemctl: edit: further reword error messages
On Wed, Feb 04, 2015 at 03:35:47AM +0300, Ivan Shapovalov wrote:
On 2015-01-05 at 17:08 +0100, Zbigniew Jędrzejewski-Szmek wrote:
On Fri, Dec 19, 2014 at 05:08:09PM +0300, Ivan Shapovalov wrote:
---
src/systemctl/systemctl.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 658793e..20c367c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -4776,7 +4776,7 @@ static int init_home_and_lookup_paths(char
**user_home, char **user_runtime, Loo
r = lookup_paths_init_from_scope(lp, arg_scope, arg_root);
if (r 0)
-return log_error_errno(r, Failed to lookup unit lookup
paths: %m);
+return log_error_errno(r, Failed to get unit lookup
paths: %m);
Maybe query? get is ugly.
OK.
return 0;
}
@@ -5900,11 +5900,11 @@ static int create_edit_temp_file(const char
*new_path, const char *original_path
r = tempfn_random(new_path, t);
if (r 0)
-return log_error_errno(r, Failed to determine temporary
filename for %s: %m, new_path);
+return log_error_errno(r, Failed to determine temporary
filename for \%s\: %m, new_path);
r = mkdir_parents(new_path, 0755);
if (r 0) {
-log_error_errno(r, Failed to create directories for %s:
%m, new_path);
+log_error_errno(r, Failed to create directories for
\%s\: %m, new_path);
free(t);
return r;
}
@@ -5913,12 +5913,12 @@ static int create_edit_temp_file(const char
*new_path, const char *original_path
if (r == -ENOENT) {
r = touch(t);
if (r 0) {
-log_error_errno(r, Failed to create temporary
file %s: %m, t);
+log_error_errno(r, Failed to create temporary
file \%s\: %m, t);
free(t);
return r;
}
} else if (r 0) {
-log_error_errno(r, Failed to copy %s to %s: %m,
original_path, t);
+log_error_errno(r, Failed to copy \%s\ to \%s\:
%m, original_path, t);
free(t);
return r;
}
@@ -6026,7 +6026,7 @@ static int unit_file_create_copy(const char
*unit_name,
if (!path_equal(fragment_path, tmp_new_path)
access(tmp_new_path, F_OK) == 0) {
char response;
-r = ask_char(response, yn, %s already exists, are
you sure to overwrite it with %s? [(y)es, (n)o] , tmp_new_path,
fragment_path);
+r = ask_char(response, yn, \%s\ already exists,
are you sure to overwrite it with \%s\? [(y)es, (n)o] , tmp_new_path,
fragment_path);
This is not gramatically correct. I think
\%s\ already exists. Overwrite with \%s\?... would be better.
OK.
if (r 0) {
free(tmp_new_path);
return r;
@@ -6040,7 +6040,7 @@ static int unit_file_create_copy(const char
*unit_name,
r = create_edit_temp_file(tmp_new_path, fragment_path,
tmp_tmp_path);
if (r 0) {
-log_error_errno(r, Failed to create temporary file for
%s: %m, tmp_new_path);
+log_error_errno(r, Failed to create temporary file for
\%s\: %m, tmp_new_path);
free(tmp_new_path);
return r;
}
@@ -6176,12 +6176,12 @@ static int edit(sd_bus *bus, char **args) {
assert(args);
if (!on_tty()) {
-log_error(Cannot edit units if we are not on a tty);
+log_error(Not on a tty, refusing.);
Why? Replacing a nice specific error message with a generic one seems to
be a step backwards.
I've tried to follow the existing pattern of most error messages in
systemd: Condition, action + -ing.
I'll reword again to make messages more specific... unless you tell me
to stop bikeshedding and leave it as is ;)
Yeah, I think that that those messages are OK.
-log_warning(Edition of %s canceled: temporary
file empty, *original);
+log_warning(Temporary file empty, not saving.);
And here.
Here? They are equally specific; editing cancelled == file not
saved.
The original one gives a hint of what the overall effect for the action is.
With you replacemnt, things are less explicit.
systemctl edit is something that may be used by relatively newbie
administrators, so some hand-holding is more useful than in other parts
of systemd.
Zbyszek
___
Re: [systemd-devel] [PATCH 3/4] systemctl: edit: further reword error messages
On 2015-01-05 at 17:08 +0100, Zbigniew Jędrzejewski-Szmek wrote:
On Fri, Dec 19, 2014 at 05:08:09PM +0300, Ivan Shapovalov wrote:
---
src/systemctl/systemctl.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 658793e..20c367c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -4776,7 +4776,7 @@ static int init_home_and_lookup_paths(char
**user_home, char **user_runtime, Loo
r = lookup_paths_init_from_scope(lp, arg_scope, arg_root);
if (r 0)
-return log_error_errno(r, Failed to lookup unit lookup
paths: %m);
+return log_error_errno(r, Failed to get unit lookup
paths: %m);
Maybe query? get is ugly.
OK.
return 0;
}
@@ -5900,11 +5900,11 @@ static int create_edit_temp_file(const char
*new_path, const char *original_path
r = tempfn_random(new_path, t);
if (r 0)
-return log_error_errno(r, Failed to determine temporary
filename for %s: %m, new_path);
+return log_error_errno(r, Failed to determine temporary
filename for \%s\: %m, new_path);
r = mkdir_parents(new_path, 0755);
if (r 0) {
-log_error_errno(r, Failed to create directories for %s:
%m, new_path);
+log_error_errno(r, Failed to create directories for
\%s\: %m, new_path);
free(t);
return r;
}
@@ -5913,12 +5913,12 @@ static int create_edit_temp_file(const char
*new_path, const char *original_path
if (r == -ENOENT) {
r = touch(t);
if (r 0) {
-log_error_errno(r, Failed to create temporary
file %s: %m, t);
+log_error_errno(r, Failed to create temporary
file \%s\: %m, t);
free(t);
return r;
}
} else if (r 0) {
-log_error_errno(r, Failed to copy %s to %s: %m,
original_path, t);
+log_error_errno(r, Failed to copy \%s\ to \%s\: %m,
original_path, t);
free(t);
return r;
}
@@ -6026,7 +6026,7 @@ static int unit_file_create_copy(const char
*unit_name,
if (!path_equal(fragment_path, tmp_new_path)
access(tmp_new_path, F_OK) == 0) {
char response;
-r = ask_char(response, yn, %s already exists, are you
sure to overwrite it with %s? [(y)es, (n)o] , tmp_new_path, fragment_path);
+r = ask_char(response, yn, \%s\ already exists, are
you sure to overwrite it with \%s\? [(y)es, (n)o] , tmp_new_path,
fragment_path);
This is not gramatically correct. I think
\%s\ already exists. Overwrite with \%s\?... would be better.
OK.
if (r 0) {
free(tmp_new_path);
return r;
@@ -6040,7 +6040,7 @@ static int unit_file_create_copy(const char
*unit_name,
r = create_edit_temp_file(tmp_new_path, fragment_path,
tmp_tmp_path);
if (r 0) {
-log_error_errno(r, Failed to create temporary file for
%s: %m, tmp_new_path);
+log_error_errno(r, Failed to create temporary file for
\%s\: %m, tmp_new_path);
free(tmp_new_path);
return r;
}
@@ -6176,12 +6176,12 @@ static int edit(sd_bus *bus, char **args) {
assert(args);
if (!on_tty()) {
-log_error(Cannot edit units if we are not on a tty);
+log_error(Not on a tty, refusing.);
Why? Replacing a nice specific error message with a generic one seems to
be a step backwards.
I've tried to follow the existing pattern of most error messages in
systemd: Condition, action + -ing.
I'll reword again to make messages more specific... unless you tell me
to stop bikeshedding and leave it as is ;)
return -EINVAL;
}
if (arg_transport != BUS_TRANSPORT_LOCAL) {
-log_error(Cannot remotely edit units);
+log_error(Remote operation requested, refusing.);
And the same here.
return -EINVAL;
}
@@ -6205,12 +6205,12 @@ static int edit(sd_bus *bus, char **args) {
* It's useful if the user wants to cancel its modification
*/
if (null_or_empty_path(*tmp)) {
-log_warning(Edition of %s canceled: temporary
file empty, *original);
+log_warning(Temporary file empty, not saving.);
And here.
Here? They are equally specific; editing cancelled == file not
saved.
--
Ivan Shapovalov / intelfx /
Re: [systemd-devel] [PATCH 3/4] systemctl: edit: further reword error messages
On Fri, Dec 19, 2014 at 05:08:09PM +0300, Ivan Shapovalov wrote:
---
src/systemctl/systemctl.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 658793e..20c367c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -4776,7 +4776,7 @@ static int init_home_and_lookup_paths(char **user_home,
char **user_runtime, Loo
r = lookup_paths_init_from_scope(lp, arg_scope, arg_root);
if (r 0)
-return log_error_errno(r, Failed to lookup unit lookup
paths: %m);
+return log_error_errno(r, Failed to get unit lookup paths:
%m);
Maybe query? get is ugly.
return 0;
}
@@ -5900,11 +5900,11 @@ static int create_edit_temp_file(const char
*new_path, const char *original_path
r = tempfn_random(new_path, t);
if (r 0)
-return log_error_errno(r, Failed to determine temporary
filename for %s: %m, new_path);
+return log_error_errno(r, Failed to determine temporary
filename for \%s\: %m, new_path);
r = mkdir_parents(new_path, 0755);
if (r 0) {
-log_error_errno(r, Failed to create directories for %s:
%m, new_path);
+log_error_errno(r, Failed to create directories for \%s\:
%m, new_path);
free(t);
return r;
}
@@ -5913,12 +5913,12 @@ static int create_edit_temp_file(const char
*new_path, const char *original_path
if (r == -ENOENT) {
r = touch(t);
if (r 0) {
-log_error_errno(r, Failed to create temporary file
%s: %m, t);
+log_error_errno(r, Failed to create temporary file
\%s\: %m, t);
free(t);
return r;
}
} else if (r 0) {
-log_error_errno(r, Failed to copy %s to %s: %m,
original_path, t);
+log_error_errno(r, Failed to copy \%s\ to \%s\: %m,
original_path, t);
free(t);
return r;
}
@@ -6026,7 +6026,7 @@ static int unit_file_create_copy(const char *unit_name,
if (!path_equal(fragment_path, tmp_new_path) access(tmp_new_path,
F_OK) == 0) {
char response;
-r = ask_char(response, yn, %s already exists, are you
sure to overwrite it with %s? [(y)es, (n)o] , tmp_new_path, fragment_path);
+r = ask_char(response, yn, \%s\ already exists, are
you sure to overwrite it with \%s\? [(y)es, (n)o] , tmp_new_path,
fragment_path);
This is not gramatically correct. I think
\%s\ already exists. Overwrite with \%s\?... would be better.
if (r 0) {
free(tmp_new_path);
return r;
@@ -6040,7 +6040,7 @@ static int unit_file_create_copy(const char *unit_name,
r = create_edit_temp_file(tmp_new_path, fragment_path,
tmp_tmp_path);
if (r 0) {
-log_error_errno(r, Failed to create temporary file for %s:
%m, tmp_new_path);
+log_error_errno(r, Failed to create temporary file for
\%s\: %m, tmp_new_path);
free(tmp_new_path);
return r;
}
@@ -6176,12 +6176,12 @@ static int edit(sd_bus *bus, char **args) {
assert(args);
if (!on_tty()) {
-log_error(Cannot edit units if we are not on a tty);
+log_error(Not on a tty, refusing.);
Why? Replacing a nice specific error message with a generic one seems to
be a step backwards.
return -EINVAL;
}
if (arg_transport != BUS_TRANSPORT_LOCAL) {
-log_error(Cannot remotely edit units);
+log_error(Remote operation requested, refusing.);
And the same here.
return -EINVAL;
}
@@ -6205,12 +6205,12 @@ static int edit(sd_bus *bus, char **args) {
* It's useful if the user wants to cancel its modification
*/
if (null_or_empty_path(*tmp)) {
-log_warning(Edition of %s canceled: temporary file
empty, *original);
+log_warning(Temporary file empty, not saving.);
And here.
continue;
}
r = rename(*tmp, *original);
if (r 0) {
-r = log_error_errno(errno, Failed to rename %s to
%s: %m, *tmp, *original);
+r = log_error_errno(errno, Failed to rename \%s\
to \%s\: %m, *tmp, *original);
goto end;
}
}
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
[systemd-devel] [PATCH 3/4] systemctl: edit: further reword error messages
---
src/systemctl/systemctl.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 658793e..20c367c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -4776,7 +4776,7 @@ static int init_home_and_lookup_paths(char **user_home,
char **user_runtime, Loo
r = lookup_paths_init_from_scope(lp, arg_scope, arg_root);
if (r 0)
-return log_error_errno(r, Failed to lookup unit lookup paths:
%m);
+return log_error_errno(r, Failed to get unit lookup paths:
%m);
return 0;
}
@@ -5900,11 +5900,11 @@ static int create_edit_temp_file(const char *new_path,
const char *original_path
r = tempfn_random(new_path, t);
if (r 0)
-return log_error_errno(r, Failed to determine temporary
filename for %s: %m, new_path);
+return log_error_errno(r, Failed to determine temporary
filename for \%s\: %m, new_path);
r = mkdir_parents(new_path, 0755);
if (r 0) {
-log_error_errno(r, Failed to create directories for %s: %m,
new_path);
+log_error_errno(r, Failed to create directories for \%s\:
%m, new_path);
free(t);
return r;
}
@@ -5913,12 +5913,12 @@ static int create_edit_temp_file(const char *new_path,
const char *original_path
if (r == -ENOENT) {
r = touch(t);
if (r 0) {
-log_error_errno(r, Failed to create temporary file
%s: %m, t);
+log_error_errno(r, Failed to create temporary file
\%s\: %m, t);
free(t);
return r;
}
} else if (r 0) {
-log_error_errno(r, Failed to copy %s to %s: %m,
original_path, t);
+log_error_errno(r, Failed to copy \%s\ to \%s\: %m,
original_path, t);
free(t);
return r;
}
@@ -6026,7 +6026,7 @@ static int unit_file_create_copy(const char *unit_name,
if (!path_equal(fragment_path, tmp_new_path) access(tmp_new_path,
F_OK) == 0) {
char response;
-r = ask_char(response, yn, %s already exists, are you sure
to overwrite it with %s? [(y)es, (n)o] , tmp_new_path, fragment_path);
+r = ask_char(response, yn, \%s\ already exists, are you
sure to overwrite it with \%s\? [(y)es, (n)o] , tmp_new_path, fragment_path);
if (r 0) {
free(tmp_new_path);
return r;
@@ -6040,7 +6040,7 @@ static int unit_file_create_copy(const char *unit_name,
r = create_edit_temp_file(tmp_new_path, fragment_path, tmp_tmp_path);
if (r 0) {
-log_error_errno(r, Failed to create temporary file for %s:
%m, tmp_new_path);
+log_error_errno(r, Failed to create temporary file for
\%s\: %m, tmp_new_path);
free(tmp_new_path);
return r;
}
@@ -6176,12 +6176,12 @@ static int edit(sd_bus *bus, char **args) {
assert(args);
if (!on_tty()) {
-log_error(Cannot edit units if we are not on a tty);
+log_error(Not on a tty, refusing.);
return -EINVAL;
}
if (arg_transport != BUS_TRANSPORT_LOCAL) {
-log_error(Cannot remotely edit units);
+log_error(Remote operation requested, refusing.);
return -EINVAL;
}
@@ -6205,12 +6205,12 @@ static int edit(sd_bus *bus, char **args) {
* It's useful if the user wants to cancel its modification
*/
if (null_or_empty_path(*tmp)) {
-log_warning(Edition of %s canceled: temporary file
empty, *original);
+log_warning(Temporary file empty, not saving.);
continue;
}
r = rename(*tmp, *original);
if (r 0) {
-r = log_error_errno(errno, Failed to rename %s to %s:
%m, *tmp, *original);
+r = log_error_errno(errno, Failed to rename \%s\ to
\%s\: %m, *tmp, *original);
goto end;
}
}
--
2.2.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
