cmd/snap: don't translate service statuses in 'snap services' output #10750
Conversation
|
I guess that's just an opinion but shouldn't scripts that rely on parsing set LANG? |
Codecov Report
@@ Coverage Diff @@
## master #10750 +/- ##
=======================================
Coverage 78.37% 78.38%
=======================================
Files 887 887
Lines 99751 99751
=======================================
+ Hits 78180 78187 +7
+ Misses 16669 16663 -6
+ Partials 4902 4901 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -138,15 +138,15 @@ func (s *svcStatus) Execute(args []string) error { | |||
| fmt.Fprintln(w, i18n.G("Service\tStartup\tCurrent\tNotes")) | |||
|
|
|||
| for _, svc := range services { | |||
| startup := i18n.G("disabled") | |||
There was a problem hiding this comment.
Hmm not sure, I would say that scripts should set LC_ALL=C or one of the other variables.
There was a problem hiding this comment.
I made this as a suggestion in the bug instead, @stolowski I'm closing this instead.
|
I would challenge you to find a tutorial on the Internet on script writing that says "change the language first" -- it doesn't happen. Part of that is because computers are American centric and that isn't good. But at a practical level what it means is that everyone's test suite is only going to run in English, and there'll be errors for everyone else on the planet. The developer won't understand it and just report the bug as WORKSFORME. Frankly, that's what my snaps do. 🤷♂️ |
|
Also, though the code is shared I'll say that I feel much more strongly about this in |
That's a very good point. But it seems to me that the code is not shared: the part for snapctl is here, and I fully agree that that part should not be using |
|
BTW, we have some code in |
Yes we use terminal.IsTerminal() check in a couple of places when it comes to formatting or escape sequences, but I fear about discoverability if it was used in this context, it could be very confusing as one usually looks at tty output first when working on a script. |
|
I'm closing this. OTOH we should take a decision whether to remove all i18n from snapctl. |
It was rightfully pointed out in https://bugs.launchpad.net/snappy/+bug/1941926 that translating service statuses makes it hard to handle 'snap services' in scripts, so this PR drops i18n around service status. I think 'snap services' should be usable for scripts just like 'systemctl' is.