GOTO Masanori wrote: > It's fine, and in this modification chance, we would like to apply > #117509 "locales: Message grammar". Do you think? Yes, that is a definite improvement. > > if db_go; then > > STATE=$(($STATE + 1)) > > else > > STATE=$(($STATE - 1)) > > fi > > done > > I wonder why this STATE transition is needed? Dennis made the config script use a state machine so it could support using debconf to back up to the previous question. This is good practice, if a bit more code. See the debconf-devel(7) man page for details. > > if [ "$1" = configure ]; then > > Trivial, "configure". Quoting or not is irrelivant if the word is [a-z]+ :-) Aside from style.. > > > > . /usr/share/debconf/confmodule > > db_version 2.0 > > > > db_get locales/locales_to_be_generated && SELECTED_LOCALES=$RET > > db_get locales/default_environment_locale && SELECTED="$RET" > > > > if [ -n "$SELECTED_LOCALES" -a "$SELECTED_LOCALES" != "None" ]; then > > if [ -e $LG ]; then > > # Comment previous defined locales > > sed -e 's/^[a-zA-Z]/#&/' $LG > $LG.tmp || true > > mv -f $LG.tmp $LG > > Why are the previous defined locales commented out, not simply removed? Cnsider a locale.gen that looks like this (a real-world example by the way): # Estoy en Honduras esta mes. es_HN ISO-8859-1 # I've been testing translations to German recently, though I don't # speak it. de_DE ISO-8859-1 de_DE@euro ISO-8859-15 The nice thing about Dennis's patch is it makes reconfiguration of locales have the same effect as if you'd edited the file. If I select an additional language, it is added on to the end. If I decide to get rid of the dated es_HN entry (not in Honduras any more), it comments it out. If it merely deleted unused entries, I would be left with this: # Estoy en Honduras esta mes. # I've been testing translations to german recently, though I don't # speak it. de_DE ISO-8859-1 de_DE@euro ISO-8859-15 No admin would leave the quote behind when removing an entry though, and this is at best ugly, at worst could be confusing. Instead the code does a good job of just commenting it out, which keeps it in the context of the comment. > > save_IFS=$IFS > > IFS=, > > for locale in $list; do > > if grep -q "^#$locale *\$" $LG; then > > # Uncomment previous defined locales > > sed -e "s,#$locale *\$,$locale," $LG > $LG.tmp || true > > mv -f $LG.tmp $LG > > else > > # Never wondered why Emacs sux? > > echo >> $LG > > Why is this "empty line" added? Because of the possibility that some text editor would neglect to add an empty line at EOF, resulting in a corrupt file after append. I suggested Dennis add something to deal with that. I'd remove the Emacs comment tho. :-) > > # Add a new locale > > echo $locale >> $LG > > fi > > done > > IFS=$save_IFS > > > > # Update requested locales. Remove all old locale dir and > > # locale-archive before generating new locale data. > > rm -rf /usr/lib/locale/* || true > > /usr/sbin/locale-gen > > fi > > > > # Set default LANG environment variable > > if [ -e $EE ]; then > > sed -e '/^ *LANG=/d' $EE > $EE.tmp || true > > else > > :> $EE.tmp > > fi > > if [ -n "$SELECTED" ] && [ "$SELECTED" != "None" ]; then > > echo "LANG=$SELECTED" >> $EE.tmp > > mv -f $EE.tmp $EE > > else > > rm -f $EE.tmp > > fi > > fi > > From my test, there was one bug. If I unselected all locales in the > first selection, but it was not reflected into /etc/locale.gen. Hmm, yes, I guess if it does not enter the for loop since the list is empty, it will not modify the file. I havn't run the code, I only eyeballed it. > It Looks really fine. I think it's ok to include in 2.3.2-1 :) -- see shy jo
Attachment:
pgpY2l6Th1__M.pgp
Description: PGP signature