[Help-smalltalk] smalltalk-2.0.5: bugs in GetText.st

[Bruno Haible]

1) This code
   PackageLoader fileInPackage: 'I18N' !
   (I18N Locale default messages domain: 'hello') printNl !
attempts to open the file
   $prefix/share/locale/de/LC_MESSAGES/hello
without .mo extension, and obviously fails.

2) You use the LcMessagesDomain variable 'cache' for two different
things.

3) You use the cache when it is nil, but you don't use it if it is non-nil.
This obviously given an error because nil doesn't understand the at:
message.

4) When "domain at: sing plural: plur" cannot find a translation, it should
return sing or plur unmodified. That's how libintl behaves.

5) The iconv transliteration doesn't work.

6) LcMessagesCatalog doesn't need to have an instance variable 'cache'
because LcMessagesDomain already has this variable.

7) Optimization: If only one .mo file exists, there is no need to create the
LcMessagesTerritoryDomain - just use the LcMessageCatalog instead.

8) The way you've programmed the caching, an LcMessagesTerritoryDomain
containing two LcMessagesCatalogs behaves essentially uncached:
The cache in the inner LcMessagesCatalogs is bypassed because you call
primAt, not at, on them. The cache in the outer LcMessagesTerritoryDomain
is nonexistent because there is no sourceCharset translation on this one.
This is also the reason for problem 5.

Here is a fix for all these, plus:
  - move primAt to the private section,
  - pluralLookup renamed to primPluralLookup, because it's similar to primAt,
  - change return value of primAt to be nil if not found, for consistency
    with primPluralLookup.


2002-09-01  Bruno Haible  <address@hidden>

        * i18n/GetText.st: Append .mo to the filename. Fix bugs related to
        caching. Make primAt private. Make pluralLookup private and rename
        it to primPluralLookup.

*** smalltalk-2.0.5/i18n/GetText.st.bak 2002-07-14 13:46:10.000000000 +0200
--- smalltalk-2.0.5/i18n/GetText.st     2002-09-01 17:07:55.000000000 +0200
***************
*** 30,35 ****
--- 30,50 ----
  |
   ======================================================================"
  
+ "Class hierarchy:
+ 
+   LocaleData
+     LocaleConventions
+       - LcMessages
+     - LcMessagesDomain
+       - LcMessagesTerritoryDomain
+       - LcMessagesDummyDomain
+       - LcMessagesCatalog
+         - LcMessagesMoFileVersion0
+   FileStream
+     - BigEndianFileStream
+   FileSegment
+     - FileStreamSegment
+ "
  
  LocaleConventions subclass: #LcMessages
      instanceVariableNames: ''
***************
*** 41,47 ****
  'This object is a factory of LcMessagesDomain objects'!
  
  LocaleData subclass: #LcMessagesDomain
!     instanceVariableNames: 'lastString lastCache cache sourceCharset'
      classVariableNames: ''
      poolDictionaries: ''
      category: 'i18n-Messages'!
--- 56,63 ----
  'This object is a factory of LcMessagesDomain objects'!
  
  LocaleData subclass: #LcMessagesDomain
!     instanceVariableNames: 'lastString lastCache cache pluralcache
!                           sourceCharset'
      classVariableNames: ''
      poolDictionaries: ''
      category: 'i18n-Messages'!
***************
*** 97,104 ****
  translated strings from a file.'!
  
  LcMessagesCatalog subclass: #LcMessagesMoFileVersion0
!     instanceVariableNames: 'cache original translated firstCharMap emptyGroup
!       pluralExpression'
      classVariableNames: 'DefaultPluralExpressions'
      poolDictionaries: ''
      category: 'i18n-Messages'!
--- 113,120 ----
  translated strings from a file.'!
  
  LcMessagesCatalog subclass: #LcMessagesMoFileVersion0
!     instanceVariableNames: 'original translated firstCharMap emptyGroup
!                           pluralExpression'
      classVariableNames: 'DefaultPluralExpressions'
      poolDictionaries: ''
      category: 'i18n-Messages'!
***************
*** 144,150 ****
  !LcMessages methodsFor: 'accessing'!
  
  territoryDirectory
!     "Answer the directory holding MO files for the territory"
      ^super territoryDirectory, '/LC_MESSAGES'!
  
  languageDirectory
--- 160,167 ----
  !LcMessages methodsFor: 'accessing'!
  
  territoryDirectory
!     "Answer the directory holding MO files for the language, specific to
!      the territory"
      ^super territoryDirectory, '/LC_MESSAGES'!
  
  languageDirectory
***************
*** 162,171 ****
  domain: aString
      "Answer an object for the aString domain, querying the language
       catalog (e.g. pt) only"
      self isPosixLocale
        ifTrue: [ ^self dummyDomain: aString ].
  
!     ^self domain: aString directory: self languageDirectory!
  
  dummyDomain
      "Answer a dummy domain that does not do a translation"
--- 179,200 ----
  domain: aString
      "Answer an object for the aString domain, querying the language
       catalog (e.g. pt) only"
+     ^self domain: aString directory: self languageDirectory!
+ 
+ domain: aString directory: dirName
+     "Answer an object for the aString domain, looking in the given
+      directory for a valid MO file."
+     | catalog |
      self isPosixLocale
        ifTrue: [ ^self dummyDomain: aString ].
  
!     catalog :=
!       LcMessagesDomain
!           id: self id
!           on: (Directory append: (aString , '.mo') to: dirName).
!     catalog isNil
!       ifFalse: [ ^catalog ]
!       ifTrue: [ ^LcMessagesDummyDomain basicNew id: self id ]!
  
  dummyDomain
      "Answer a dummy domain that does not do a translation"
***************
*** 179,202 ****
      "Answer an object for the aString domain, querying both the
       language catalog (e.g. pt) and the territory catalog (e.g. pt_BR
       or pt_PT)."
      self isPosixLocale
        ifTrue: [ ^self dummyDomain: aString ].
  
      ^LcMessagesTerritoryDomain basicNew
        id: self id;
!       primary: (self domain: aString directory: self territoryDirectory);
!       secondary: (self domain: aString directory: self languageDirectory);
        yourself!
! 
! domain: aString directory: dirName
!     "Answer an object for the aString domain, looking in the given
!      directory for a valid MO file."
!     self isPosixLocale
!       ifTrue: [ ^self dummyDomain: aString ].
! 
!     ^LcMessagesDomain
!       id: self id
!       on: (Directory append: aString to: dirName)! !
  
  
  !LcMessagesDomain class methodsFor: 'opening MO files'!
--- 208,239 ----
      "Answer an object for the aString domain, querying both the
       language catalog (e.g. pt) and the territory catalog (e.g. pt_BR
       or pt_PT)."
+     | primarycatalog secondarycatalog count |
      self isPosixLocale
        ifTrue: [ ^self dummyDomain: aString ].
  
+     primarycatalog :=
+       self domain: aString directory: self territoryDirectory.
+     secondarycatalog :=
+       self domain: aString directory: self languageDirectory.
+ 
+     "If we got only one catalog, just use it. If we got more than one
+      catalog, combine them through a LcMessagesTerritoryDomain."
+     count := (primarycatalog notNil ifTrue: [ 1 ] ifFalse: [ 0 ]) +
+            (secondarycatalog notNil ifTrue: [ 1 ] ifFalse: [ 0 ]).
+     (count = 0) ifTrue: [ ^LcMessagesDummyDomain basicNew id: self id ].
+     (count = 1) ifTrue: [
+       primarycatalog notNil ifTrue: [ ^primarycatalog ].
+       ^secondarycatalog
+     ].
      ^LcMessagesTerritoryDomain basicNew
        id: self id;
!       primary: (primarycatalog notNil ifTrue: [ primarycatalog ]
!                 ifFalse: [ LcMessagesDummyDomain basicNew id: self id ])
!       secondary: (secondarycatalog notNil ifTrue: [ secondarycatalog ]
!                   ifFalse: [ LcMessagesDummyDomain basicNew id: self id ])
        yourself!
! !
  
  
  !LcMessagesDomain class methodsFor: 'opening MO files'!
***************
*** 227,246 ****
            LcMessagesMoFileVersion0 basicNew
                id: anArray;
                initialize: stream ]
!       ifFalse: [
!           LcMessagesDummyDomain basicNew id: anArray ]! !
  
  !LcMessagesDomain methodsFor: 'querying'!
  
  at: aString
      "Answer the translation of `aString', or answer aString itself
       if none is available."
!     ^self? aString!
  
  at: singularString plural: pluralString with: n
      "Answer either the translation of pluralString with `%1' replaced by
       n if n ~= 1, or the translation of singularString if n = 1."
!     | composedString plural translit |
      (composedString := String new: singularString size + pluralString size + 
1)
        replaceFrom: 1
            to: singularString size
--- 264,316 ----
            LcMessagesMoFileVersion0 basicNew
                id: anArray;
                initialize: stream ]
!       ifFalse: [ nil ]! !
  
  !LcMessagesDomain methodsFor: 'querying'!
  
+ ? aString
+     "Answer the translation of `aString', or answer aString itself
+      if none is available."
+     ^self at: aString!
+ 
  at: aString
      "Answer the translation of `aString', or answer aString itself
       if none is available."
!     | translation |
!     aString == lastString ifFalse: [
!       lastString := aString.
!       "Check the cache first"
!       translation := nil.
!       cache isNil ifFalse: [
!           translation := cache at: aString ifAbsent: [ nil ].
!       ].
!       translation isNil ifTrue: [
!           translation := self primAt: aString.
!           translation isNil
!               ifTrue: [ translation := aString ]
!               ifFalse: [
!                   "Check whether we must transliterate the translation.
!                    Note that if we go through the transliteration we
!                    automatically build a cache." 
!                   sourceCharset notNil ifTrue: [
!                       translation := (EncodedStream
!                           on: translation
!                           from: sourceCharset
!                           to: self charset) contents
!                   ]
!               ].
!       ].
!       cache isNil ifFalse: [
!           cache at: aString put: translation.
!       ].
!       lastCache := translation.
!     ].
!     ^lastCache!
  
  at: singularString plural: pluralString with: n
      "Answer either the translation of pluralString with `%1' replaced by
       n if n ~= 1, or the translation of singularString if n = 1."
!     | composedString translation translit |
      (composedString := String new: singularString size + pluralString size + 
1)
        replaceFrom: 1
            to: singularString size
***************
*** 254,318 ****
            with: pluralString
            startingAt: 1.
  
!     plural := self pluralLookup: composedString with: n.
!     plural isNil ifTrue: [
!       plural := n = 1
!           ifTrue: [ self ? singularString ]
!           ifFalse: [ self ? pluralString ]
!     ].
!     sourceCharset isNil ifTrue: [ ^plural ].
! 
!     "Check the cache first"
!     cache isNil ifTrue: [
!         translit := cache at: plural ifAbsent: [ nil ].
!         translit isNil ifFalse: [ ^translit ].
!     ].
!     translit := (EncodedStream
!       on: plural
!       from: sourceCharset
!       to: self charset) contents.
! 
!     cache isNil ifFalse: [
!         cache at: plural put: translit.
      ].
-     ^translit
- !
  
! at: aString put: anotherString
!     self shouldNotImplement!
! 
! ? aString
!     "Answer the translation of `aString', or answer aString itself
!      if none is available."
!     aString == lastString ifFalse: [
!       lastString := aString.
!       "Check the cache first"
!       cache isNil ifTrue: [
!           lastCache := cache at: aString ifAbsent: [ nil ].
!           lastCache isNil ifFalse: [ ^lastCache ].
        ].
!       lastCache := self primAt: aString.
! 
!       "Check whether we must transliterate the translation.
!        Note that if we go through the transliteration we automatically
!        build a cache." 
!       sourceCharset notNil ifTrue: [
!           lastCache := (EncodedStream
!               on: lastCache
                from: sourceCharset
!               to: self charset) contents
        ].
!       cache isNil ifFalse: [
!           cache at: aString put: lastCache.
        ].
      ].
-     ^lastCache!
  
! primAt: aString
!     "Answer the translation of `aString', or answer aString itself
!      if none is available.  This sits below the caching and transli-
!      teration operated by #?."
!     self subclassResponsibility!
  
  translatorInformation
      "Answer information on the translation, or nil if there is none.
--- 324,357 ----
            with: pluralString
            startingAt: 1.
  
!     translation := self pluralLookup: composedString with: n.
!     translation isNil ifTrue: [
!       ^n = 1 ifTrue: [ singularString ] ifFalse: [ pluralString ]
      ].
  
!     "Check whether we must transliterate the translation."
!     sourceCharset notNil ifTrue: [
!       translit := nil.
!       pluralcache isNil ifFalse: [
!           translit := pluralcache at: translation ifAbsent: [ nil ].
        ].
!       translit isNil ifTrue: [
!           translit := (EncodedStream
!               on: translation
                from: sourceCharset
!               to: self charset) contents.
        ].
!       pluralcache isNil ifFalse: [
!           pluralcache at: translation put: translit.
        ].
+       translation := translit.
      ].
  
!     ^translation
! !
! 
! at: aString put: anotherString
!     self shouldNotImplement!
  
  translatorInformation
      "Answer information on the translation, or nil if there is none.
***************
*** 320,326 ****
  
      | info |
      info := self primAt: ''.
-     info isEmpty ifTrue: [ info := nil ].
      ^info
  !
  
--- 359,364 ----
***************
*** 375,391 ****
      ].
  
      self shouldCache ifTrue: [
!       cache := LookupTable new ].
  ! !
  
! !LcMessagesDomain methodsFor: 'private - plurals'!
  
! pluralLookup: composedString with: n
      "Answer a translation of composedString (two nul-separated strings
       with the English singular and plural) valid when %1 is replaced 
!      with `n', or nil if none could be found."
!     ^nil
! ! !
  
  
  !LcMessagesTerritoryDomain class methodsFor: 'instance creation'!
--- 413,437 ----
      ].
  
      self shouldCache ifTrue: [
!       cache := LookupTable new.
!       pluralcache := LookupTable new. ].
  ! !
  
! !LcMessagesDomain methodsFor: 'private'!
! 
! primAt: aString
!     "Answer the translation of `aString', or answer nil
!      if none is available.  This sits below the caching and
!      transliteration operated by #?."
!     self subclassResponsibility!
  
! primPluralLookup: composedString with: n
      "Answer a translation of composedString (two nul-separated strings
       with the English singular and plural) valid when %1 is replaced 
!      with `n', or nil if none could be found.  This sits below the
!      caching and transliteration layer."
!     self subclassResponsibility!
! !
  
  
  !LcMessagesTerritoryDomain class methodsFor: 'instance creation'!
***************
*** 395,435 ****
       domain1 and domain2"
      ^self new primary: domain1 secondary: domain2! !
  
! !LcMessagesTerritoryDomain methodsFor: 'querying'!
  
  primAt: aString
!     "Answer the translation of `aString', or answer aString itself
!      if none is available.  This sits below the caching and transli-
!      teration operated by #?."
!     | primaryTranslation |
!     ^(primaryTranslation := primary primAt: aString) == aString
!       ifTrue: [ secondary primAt: aString ]
!       ifFalse: [ primaryTranslation ]!
  
! pluralLookup: composedString with: n
      "Answer a translation of composedString (two nul-separated strings
       with the English singular and plural) valid when %1 is replaced 
!      with `n', or nil if none could be found."
      | primaryTranslation |
      primaryTranslation := primary pluralLookup: composedString with: n.
  
      ^primaryTranslation isNil
        ifTrue: [ secondary pluralLookup: composedString with: n ]
!       ifFalse: [ primaryTranslation ]! !
! 
! !LcMessagesTerritoryDomain methodsFor: 'private'!
  
! primary: domain1 secondary: domain2
!     primary := domain1.
!     secondary := domain2! !
  
! !LcMessagesDummyDomain methodsFor: 'querying'!
  
  primAt: aString
!     "Answer the translation of `aString', or answer aString itself
       if none is available (which always happens in this class)."
!     ^aString! !
  
  
  !LcMessagesCatalog methodsFor: 'private'!
  
--- 441,503 ----
       domain1 and domain2"
      ^self new primary: domain1 secondary: domain2! !
  
! !LcMessagesTerritoryDomain methodsFor: 'private'!
! 
! primary: domain1 secondary: domain2
!     primary := domain1.
!     secondary := domain2.
!     self flush!
! 
! primary
!     "Answer the first domain"
!     ^primary!
! 
! secondary
!     "Answer the second domain"
!     ^secondary!
  
  primAt: aString
!     "Answer the translation of `aString', or answer nil
!      if none is available.  This sits below the caching and
!      transliteration operated by #?."
!     | translation |
!     translation := primary primAt: aString.
!     translation isNil ifFalse: [ ^translation ].
!     secondary primAt: aString!
  
! primPluralLookup: composedString with: n
      "Answer a translation of composedString (two nul-separated strings
       with the English singular and plural) valid when %1 is replaced 
!      with `n', or nil if none could be found.  This sits below the
!      caching and transliteration layer."
      | primaryTranslation |
      primaryTranslation := primary pluralLookup: composedString with: n.
  
      ^primaryTranslation isNil
        ifTrue: [ secondary pluralLookup: composedString with: n ]
!       ifFalse: [ primaryTranslation ]!
  
! shouldCache
!     "Answer whether translations should be cached"
!     "Yes we cache them here because we bypass the caches in primary
!      and secondary."
!     ^true!
! !
  
! !LcMessagesDummyDomain methodsFor: 'private'!
  
  primAt: aString
!     "Answer the translation of `aString', or answer nil
       if none is available (which always happens in this class)."
!     ^nil!
  
+ primPluralLookup: composedString with: n
+     "Answer a translation of composedString (two nul-separated strings
+      with the English singular and plural) valid when %1 is replaced 
+      with `n', or nil if none could be found (which always happens in
+      this class)."
+     ^nil!
+ !
  
  !LcMessagesCatalog methodsFor: 'private'!
  
***************
*** 663,729 ****
      super flush.
  ! !
  
- !LcMessagesMoFileVersion0 methodsFor: 'plurals'!
- 
- pluralLookup: composedString with: n
-     "Answer a translation of composedString (two nul-separated strings
-      with the English singular and plural) valid when %1 is replaced 
-      with `n', or nil if none could be found."
-     | pluralStrings index endIndex |
- 
-     "Possible bug!! Transliterating at this point could
-      cause bugs if nuls are massaged (e.g. UTF7, UCS4)."
-     pluralStrings := self primAt: composedString.
-     pluralStrings == composedString ifTrue: [ ^nil ].
- 
-     "Find the start of the string in the composed string"
-     index := 1.
-     (self pluralLookup: n) timesRepeat: [
-       index := 1 + (pluralStrings
-           indexOf: Character nul
-           startingAt: index
-           ifAbsent: [ pluralStrings size + 1 ])
-     ].
- 
-     "Find the end of the string in the composed string"
-     index > pluralStrings size ifFalse: [
-       endIndex := (pluralStrings
-           indexOf: Character nul
-           startingAt: index
-           ifAbsent: [ pluralStrings size + 1 ]) - 1.
- 
-       ^pluralStrings copyFrom: index to: endIndex
-     ].
- 
-     "Fallback case, use standard rule for Germanic languages."
-     ^nil
- ! !
- 
  !LcMessagesMoFileVersion0 methodsFor: 'private'!
  
- pluralLookup: n
-     "Answer the index of the plural form that must be used for a value
-      of `n'."
-     ^self pluralExpression value: n
- !
- 
- pluralExpression
-     "Answer a RunTimeExpression which picks the correct plural
-      form for the catalog"
-     | config |
-     pluralExpression isNil ifFalse: [ ^pluralExpression ].
-     config := self translatorInformationAt: 'Plural-Forms' at: 'plural'.
-     pluralExpression := config isNil
-       ifTrue: [
-           self class
-               pluralExpressionFor: self
-               ifAbsent: [ RunTimeExpression on: '(n != 1)' ]
-       ]
-       ifFalse: [ RunTimeExpression on: config ].
- 
-     ^pluralExpression
- !
- 
  getFirstChars
      "This implementation does a limited form of bucketing
       to supply the speed lost by not implementing hashing. This
--- 731,738 ----
***************
*** 781,801 ****
      ]
  !
  
- primAt: aString
-     "Translate aString, answer the translation"
-     | group n |
-     group := aString isEmpty
-       ifTrue: [ emptyGroup ]
-       ifFalse: [ firstCharMap at: (aString at: 1) value + 1 ].
- 
-     group isNil ifTrue: [ ^aString ].
- 
-     n := self binarySearch: aString from: group first to: group last.
-     ^n isNil
-       ifTrue: [ aString ]
-       ifFalse: [ (translated at: n) asString ]
- !
- 
  binarySearch: aString from: low to: high
      "Do a binary search on `original', searching for aString"
      | i j mid originalString result |
--- 790,795 ----
***************
*** 816,822 ****
            ifFalse: [ i := mid + 1 ].
      ].
      ^nil
! ! !
  
  
  !BigEndianFileStream methodsFor: 'private - endianness switching'!
--- 810,889 ----
            ifFalse: [ i := mid + 1 ].
      ].
      ^nil
! !
! 
! primAt: aString
!     "Translate aString, answer the translation or nil"
!     | group n |
!     group := aString isEmpty
!       ifTrue: [ emptyGroup ]
!       ifFalse: [ firstCharMap at: (aString at: 1) value + 1 ].
! 
!     group isNil ifTrue: [ ^nil ].
! 
!     n := self binarySearch: aString from: group first to: group last.
!     ^n isNil
!       ifTrue: [ nil ]
!       ifFalse: [ (translated at: n) asString ]
! !
! 
! primPluralLookup: composedString with: n
!     "Answer a translation of composedString (two nul-separated strings
!      with the English singular and plural) valid when %1 is replaced 
!      with `n', or nil if none could be found.  This sits below the
!      caching and transliteration layer."
!     | pluralStrings index endIndex |
! 
!     "Possible bug!! Transliterating at this point could
!      cause bugs if nuls are massaged (e.g. UTF7, UCS4)."
!     pluralStrings := self primAt: composedString.
!     pluralStrings isNil ifTrue: [ ^nil ].
! 
!     "Find the start of the string in the composed string"
!     index := 1.
!     (self pluralLookup: n) timesRepeat: [
!       index := 1 + (pluralStrings
!           indexOf: Character nul
!           startingAt: index
!           ifAbsent: [ pluralStrings size + 1 ])
!     ].
! 
!     "Find the end of the string in the composed string"
!     index > pluralStrings size ifFalse: [
!       endIndex := (pluralStrings
!           indexOf: Character nul
!           startingAt: index
!           ifAbsent: [ pluralStrings size + 1 ]) - 1.
! 
!       ^pluralStrings copyFrom: index to: endIndex
!     ].
! 
!     "Fallback case, use standard rule for Germanic languages."
!     ^nil!
! 
! pluralLookup: n
!     "Answer the index of the plural form that must be used for a value
!      of `n'."
!     ^self pluralExpression value: n
! !
! 
! pluralExpression
!     "Answer a RunTimeExpression which picks the correct plural
!      form for the catalog"
!     | config |
!     pluralExpression isNil ifTrue: [
!       config := self translatorInformationAt: 'Plural-Forms' at: 'plural'.
!       pluralExpression := config isNil
!           ifFalse: [ RunTimeExpression on: config ]
!           ifTrue: [
!               self class
!                   pluralExpressionFor: self
!                   ifAbsent: [ RunTimeExpression on: '(n != 1)' ]
!           ]
!     ].
!     ^pluralExpression
! !
! !
  
  
  !BigEndianFileStream methodsFor: 'private - endianness switching'!
***************
*** 872,885 ****
       moved past the signature."
      | magic |
      le size = be size ifFalse: [
!         self error: 'mismatching sizes for big-endian and little-endian' ].
  
      magic := (self next: le size) asByteArray.
      magic = be ifTrue: [
!         self changeClassTo: I18N BigEndianFileStream ].
      magic = le ifFalse: [
!         self skip: le size negated.
!         self error: 'mismatching magic number' ].
  ! !
  
  LcMessagesMoFileVersion0 initialize!
--- 939,952 ----
       moved past the signature."
      | magic |
      le size = be size ifFalse: [
!       self error: 'mismatching sizes for big-endian and little-endian' ].
  
      magic := (self next: le size) asByteArray.
      magic = be ifTrue: [
!       self changeClassTo: I18N BigEndianFileStream ].
      magic = le ifFalse: [
!       self skip: le size negated.
!       self error: 'mismatching magic number' ].
  ! !
  
  LcMessagesMoFileVersion0 initialize!