Re: [Midnight Commander] #50: savannah: localized headers in .mc/history

["Ticket System" <tickets midnight-commander org>]

#50: savannah: localized headers in .mc/history
--------------------------+-------------------------------------------------
  Reporter:  slavazanko   |       Owner:  slavazanko                      
      Type:  enhancement  |      Status:  testing                         
  Priority:  major        |   Milestone:  4.6.2                           
 Component:  mc-core      |     Version:                                  
Resolution:  fixed        |    Keywords:  vote-winnie vote-slyfox approved
  Blocking:               |   Blockedby:                                  
--------------------------+-------------------------------------------------
Changes (by metux):

  * status:  accepted => testing
  * resolution:  => fixed


Old description:

> Original: http://savannah.gnu.org/bugs/?17269
>
> ||Submitted by:||Egmont Koblinger <egmont>||Submitted on:||Wed 02 Aug
> 2006 12:48:15 PM UTC||
> ||Category:||Core||Severity:||3 - Normal||
> ||Status:||None||Privacy:||Public||
> ||Assigned to:||None||Open/Closed:||Open||
> ||Release:||current (CVS or snapshot)||Operating System:||GNU/Linux||
>
> Discussion:
> {{{
> Tue 19 Sep 2006 08:24:44 AM UTC, comment #7:
>
> I am attaching a patch which adds a new argument to
> fg_input_dialog_help() - the history section name. I've adjusted
> all callers (hopefully) to pass the new argument. Please, test the
> patch. The patch should be applied against latest CVS.
>
> This patch doesn't attemt to change the section names. However, the
> current choice for section names is really bad, IMHO. Maybe now is
> the time to rename those names for good. I'd like to hear your
> opinion on that matter too.
>         Pavel Tsekov <ptsekov>
> Project Administrator
> Wed 13 Sep 2006 01:55:14 PM UTC, comment #6:
>
> I think I'll proceed as following:
>
> 1) Currently fg_input_dialog_help() assumes that it is passed
> translated strings. I won't change that because some of the callers
> pass as an argument a string which is the result of sprintf() on
> translated template string. However, I'll change
> fg_input_dialog_help() to prevent the translation of the already
> translated strings. Additionally I'll add comments to indicate that
> fg_input_dialog_help() and any wrappers must take translated
> strings.
>
> 2) Next I'll change fg_input_dialog_help() to take a new argument
> - the history section name. Then I will adjust all callers.
>         Pavel Tsekov <ptsekov>
> Project Administrator
> Wed 13 Sep 2006 12:38:16 PM UTC, comment #5:
>
> My last comment may be slightly misleading. To clarify:
>
> nice_cd() and others use fg_input_dialog_help() which takes
> care of displaying the input dialog and derives history file
> section names from the input dialog heading.
>         Pavel Tsekov <ptsekov>
> Project Administrator
> Wed 13 Sep 2006 10:00:24 AM UTC, comment #4:
>
> Localizing an already localized string definitely shows wrong
> design. Theoretically it can lead to false strings appearing on the
> screen (if the first translated string happens to be the same as an
> English string used somewhere else in mc). It's quite unlikely for
> this bug to ever occur, but still it's better to avoid it by good
> design. It'd be bad if the person first hitting this bug would have
> to choose different wording or if he had to fix mc in this respect.
>
> (Theoretically :-)) each function has a documented interface of
> what the meaning of its arguments are, and this documentation
> should state for all strings whether they are localized or not.
>
> You say: "this may not be the best solution since the programmer
> must know that it should call nice_cd () with N_() instead of ()".
> I think this approach is wrong. Every programmer wishing to use an
> already implemented function should first check the docs or the
> comments in the code to see what this function expects and then use
> it accordingly. If there's no docs stating whether the argument to
> nice_cd should be localized or not then _this is a bug in mc. If
> there are docs, however, then there's nothing you can do against
> wrong usage and it's not your problem anymore.
>
> (BTW it starts resembling the usually-misunderstood Hungarian
> notation by Simonyi, see:
> http://www.joelonsoftware.com/articles/Wrong.html
> for the right explanation. If strings are always stored in
> variables whose name begins with a prefix stating whether it's
> already localized, to be localized, or non-translateable, then
> these kinds of bugs are less likely to occur.)
>
> About your first suggestion: it avoids translating an already
> translated string, hence it should really be done.
>
> About your second suggestion: it could make the config file look
> the same even if for some reason the English UI string is modified.
> Hence it is a nice move, too.
>
> So, I recommend making both changes :-)
>         Egmont Koblinger <egmont>
> Wed 13 Sep 2006 09:28:22 AM UTC, comment #3:
>
> I looked at that problem and the solution is pretty easy. Still I'd
> like to discuss the issue with anyone interested so we can find the
> best way to solve the problem.
>
> Most of the problematic entries are created by an invocation of the
> function nice_cd() located in src/cmd.c . Its first argument is a
> string to be used as the heading of the dialog. The same string is
> also used to form the name of the section in the history file where
> entries fed to the input field of the dialog will be stored. A
> typical call to nice_cd() looks like this:
>
> nice_cd (_("Title string"), ...)
>
> So as you can see nice_cd() is passed a translated string - this is
> why a translated section name is written to the history file.
> However this is unnecessary since later the title string is
> translated one more time when quick_dialog() is invoked to show the
> dialog. So, one way to solve the problem is to change nice_cd()
> invocations like this:
>
> nice_cd (N_("Title string"), ...)
>
> However, this may not be the best solution since the programmer
> must know that it should call nice_cd () with N_() instead of _().
> So, maybe a better way to solve the problem would be to add
>
> a new argument to nice_cd() which will hold the history section name.
>
> Any thoughts ?
>         Pavel Tsekov <ptsekov>
> Project Administrator
> Wed 02 Aug 2006 02:09:08 PM UTC, comment #2:
>
> Maybe once... not now, I don't have time for this nowadays.
>         Egmont Koblinger <egmont>
> Wed 02 Aug 2006 01:18:32 PM UTC, comment #1:
>
> I agree with this. Can you come up with a patch?
>         Leonard den Ottolander <leonardjo>
> Project Member
> Wed 02 Aug 2006 12:48:15 PM UTC, original submission:
>
> .mc/history contains headers like this:
> [inp Kijelölés ]
> [inpÚj könyvtár létrehozása]
> and so on...
>
> Localizing these entries may cause troubles:
> - entries in the history are not available if for some reason you
> temporarily start mc with another locale
> - entries get lost if mc is upgraded and the translators happened
> to modify the translation
>
> Headers in the history file should be fixed (non-translateable)
> English strings.
> }}}

New description:

 Original: http://savannah.gnu.org/bugs/?17269

 ||Submitted by:||Egmont Koblinger <egmont>||Submitted on:||Wed 02 Aug 2006
 12:48:15 PM UTC||
 ||Category:||Core||Severity:||3 - Normal||
 ||Status:||None||Privacy:||Public||
 ||Assigned to:||None||Open/Closed:||Open||
 ||Release:||current (CVS or snapshot)||Operating System:||GNU/Linux||

 Discussion:
 {{{
 Tue 19 Sep 2006 08:24:44 AM UTC, comment #7:

 I am attaching a patch which adds a new argument to
 fg_input_dialog_help() - the history section name. I've adjusted
 all callers (hopefully) to pass the new argument. Please, test the
 patch. The patch should be applied against latest CVS.

 This patch doesn't attemt to change the section names. However, the
 current choice for section names is really bad, IMHO. Maybe now is
 the time to rename those names for good. I'd like to hear your
 opinion on that matter too.
         Pavel Tsekov <ptsekov>
 Project Administrator
 Wed 13 Sep 2006 01:55:14 PM UTC, comment #6:

 I think I'll proceed as following:

 1) Currently fg_input_dialog_help() assumes that it is passed
 translated strings. I won't change that because some of the callers
 pass as an argument a string which is the result of sprintf() on
 translated template string. However, I'll change
 fg_input_dialog_help() to prevent the translation of the already
 translated strings. Additionally I'll add comments to indicate that
 fg_input_dialog_help() and any wrappers must take translated
 strings.

 2) Next I'll change fg_input_dialog_help() to take a new argument
 - the history section name. Then I will adjust all callers.
         Pavel Tsekov <ptsekov>
 Project Administrator
 Wed 13 Sep 2006 12:38:16 PM UTC, comment #5:

 My last comment may be slightly misleading. To clarify:

 nice_cd() and others use fg_input_dialog_help() which takes
 care of displaying the input dialog and derives history file
 section names from the input dialog heading.
         Pavel Tsekov <ptsekov>
 Project Administrator
 Wed 13 Sep 2006 10:00:24 AM UTC, comment #4:

 Localizing an already localized string definitely shows wrong
 design. Theoretically it can lead to false strings appearing on the
 screen (if the first translated string happens to be the same as an
 English string used somewhere else in mc). It's quite unlikely for
 this bug to ever occur, but still it's better to avoid it by good
 design. It'd be bad if the person first hitting this bug would have
 to choose different wording or if he had to fix mc in this respect.

 (Theoretically :-)) each function has a documented interface of
 what the meaning of its arguments are, and this documentation
 should state for all strings whether they are localized or not.

 You say: "this may not be the best solution since the programmer
 must know that it should call nice_cd () with N_() instead of ()".
 I think this approach is wrong. Every programmer wishing to use an
 already implemented function should first check the docs or the
 comments in the code to see what this function expects and then use
 it accordingly. If there's no docs stating whether the argument to
 nice_cd should be localized or not then _this is a bug in mc. If
 there are docs, however, then there's nothing you can do against
 wrong usage and it's not your problem anymore.

 (BTW it starts resembling the usually-misunderstood Hungarian
 notation by Simonyi, see:
 http://www.joelonsoftware.com/articles/Wrong.html
 for the right explanation. If strings are always stored in
 variables whose name begins with a prefix stating whether it's
 already localized, to be localized, or non-translateable, then
 these kinds of bugs are less likely to occur.)

 About your first suggestion: it avoids translating an already
 translated string, hence it should really be done.

 About your second suggestion: it could make the config file look
 the same even if for some reason the English UI string is modified.
 Hence it is a nice move, too.

 So, I recommend making both changes :-)
         Egmont Koblinger <egmont>
 Wed 13 Sep 2006 09:28:22 AM UTC, comment #3:

 I looked at that problem and the solution is pretty easy. Still I'd
 like to discuss the issue with anyone interested so we can find the
 best way to solve the problem.

 Most of the problematic entries are created by an invocation of the
 function nice_cd() located in src/cmd.c . Its first argument is a
 string to be used as the heading of the dialog. The same string is
 also used to form the name of the section in the history file where
 entries fed to the input field of the dialog will be stored. A
 typical call to nice_cd() looks like this:

 nice_cd (_("Title string"), ...)

 So as you can see nice_cd() is passed a translated string - this is
 why a translated section name is written to the history file.
 However this is unnecessary since later the title string is
 translated one more time when quick_dialog() is invoked to show the
 dialog. So, one way to solve the problem is to change nice_cd()
 invocations like this:

 nice_cd (N_("Title string"), ...)

 However, this may not be the best solution since the programmer
 must know that it should call nice_cd () with N_() instead of _().
 So, maybe a better way to solve the problem would be to add

 a new argument to nice_cd() which will hold the history section name.

 Any thoughts ?
         Pavel Tsekov <ptsekov>
 Project Administrator
 Wed 02 Aug 2006 02:09:08 PM UTC, comment #2:

 Maybe once... not now, I don't have time for this nowadays.
         Egmont Koblinger <egmont>
 Wed 02 Aug 2006 01:18:32 PM UTC, comment #1:

 I agree with this. Can you come up with a patch?
         Leonard den Ottolander <leonardjo>
 Project Member
 Wed 02 Aug 2006 12:48:15 PM UTC, original submission:

 .mc/history contains headers like this:
 [inp Kijelölés ]
 [inpÚj könyvtár létrehozása]
 and so on...

 Localizing these entries may cause troubles:
 - entries in the history are not available if for some reason you
 temporarily start mc with another locale
 - entries get lost if mc is upgraded and the translators happened
 to modify the translation

 Headers in the history file should be fixed (non-translateable)
 English strings.
 }}}

 committed to master:

 changeset:7cef5b112ea9015422882a070fa12a01c23ce5a5

--

-- 
Ticket URL: <www.midnight-commander.org/ticket/50#comment:11>
Midnight Commander <www.midnight-commander.org>
Midnight Development Center