[Home]

Summary:ASTERISK-11877: [patch] message about number of new and old messages not properly conjugated in Russian
Reporter:David Chappell (chappell)Labels:
Date Opened:2008-04-18 09:35:39Date Closed:2009-03-29 12:48:06
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_say_counted.c
( 1) app_voicemail.c.diff
( 2) vm_multilang.patch
( 3) vm_univ_ru.diff
Description:This is an old patch which we have been using for over a year.  Without it the introductory message about how many voicemail messages are in the new and old boxes is garbled.  The problems include dropt works, bad grammar, and wrong counts.  As best I remember the patch fixes these problems:

* removed bad exception code for the Russian word for "one" in the neuter singular
* due to confusion one of the counts of messages was wrong
* fixed conjugation of the adjectives meaning "old" and "new"

If necessary, I can go back and study the old code in order to accurately describe exactly how it garbles the message.


Comments:By: Joshua C. Colp (jcolp) 2008-04-18 10:01:01

This patch doesn't apply at all to the current 1.4 tree, can you please update it.

As well you change get_lastdigits but this function is also used in vm_intro_ua, causing the build to fail.

By: David Chappell (chappell) 2008-04-18 10:01:37

By the way, I notice that most languages provide their own version of vm_intro().  I believe the need for this could be eliminated for most if not all languages by improving the default (vm_intro_en()) function and adding general use functions for it to call which declining nouns and adjectives in counting contexts.

Basically the non-English languages provide their own versions in order to correct three deficiencies in the default function:

1) It contains a hard coded rule for declining (in this case pluralizing) the word "message" rather than calling function which takes a language parameter and count and selects the proper noun form.

2) It does not attempt to decline the words "old" and "new" since they are undeclinable in English.

3) It plays the names of mailboxes (vm-Old and vm-INBOX rather than the words "old" and "new".  This problem is masked in English by the fact that the default sound files vm-Old and vm-INBOX contain only the words "old" and "new" respectively.  In other languages this won't work.

This last problem will become evident to English speakers if they re-record the sound files to contain the phrases "the new messages" and "the old messages".  You will hear "You have 5 the new messages and 10 the old messages messages."  When non-English speakers hear this gibberish they start creating their own function.

I am not trying to include these problems in this bug report.  I am just trying to give this bug some context.  If anyone wants to discuss these additional issues, please contact me directly at David.Chappell@trincoll.edu.

By: Tilghman Lesher (tilghman) 2008-04-18 13:46:46

What is really needed is a document that describes what each prompt should contain, since it is not at all obvious for anything but the 3 languages that we distribute sounds for:  English, Spanish, and French.

If you can put together such a document, that would be most helpful.  It should include the module name required for each prompt, the base sound name (including any leading subdirectory names), and enough script to allow a native speaker to record their own prompts.

When ready, we will probably add it in doc/lang/ru.txt.

By: David Chappell (chappell) 2008-05-02 11:38:38

I missed the fact that somebody copied the Russian greeting function, fixed some of the bugs, and made a Ukrainian function out of it.  Plus, somebody else got in a fix for some (but not all) of the Russian bugs a day or two before I did.  I have a corrected patch which I will post shortly.

I would be happy to prepare a document explaining how to record sound files for other languages, but first we need to provide a language-neutral version of the voicemail greeting and mailbox name functions.  Otherwise the document would just say, "copy-and-paste the English functions and hack away!"

The voicemail greeting function would need two new functions in say.c.  These functions would put language-dependent endings on nouns and adjectives when counting things.  For example, the function for nouns would add "s" for two and above.  (For some languages the rules are much more complicated.)  I believe there is a Polish-only implementation of some of this in Asterisk-Addons.

I am preparing a more radical version of my patch which provides a language-neutral version of vm_intro() and say.c functions with word ending rules for English and Russian.  My patch would switch Russian over to the language-neutral functions.

Though my patch would not be in the code path for any language other than Russian I am afraid it might be to radical to be accepted simply because it touches three files: app_voicemail.c, say.c, and say.h.  Any thoughts?

By: Tilghman Lesher (tilghman) 2008-06-03 17:28:48

chappell: I'd need to see the patch before I can approve or disapprove of any particular approach.

By: Tilghman Lesher (tilghman) 2008-06-19 17:58:46

chappell: is this a work in progress?

By: David Chappell (chappell) 2008-06-25 11:35:31

Yes, this is a work in progress.  We are currently testing the form of the patch which is previously described as "more radical".  I will post later this week so that you can comment on it.

By: David Chappell (chappell) 2008-07-07 14:10:46

I have posted a new patch which represents the work-in-progress.  The patch file to which I refer is vm_univ_rm.diff.  The patch is against Asterisk 1.4.21.1

This patch provides a general solution to the problem of counting things and uses this solution to tell the caller how many messages he has.

In more detail:

* new functions in say.c which take a base sound file name and a count and attach the proper ending (in English nothing or 's')
* vm_intro_universal() which uses the new functions in say.c.  At this point, this function is used only for Russian
* Dialplan applications which expose the new functions in say.c as SayCountedNoun and SayCountedAdj

By: Tilghman Lesher (tilghman) 2008-07-07 17:48:03

I need you to take out all of the inline comments that specify which sound files are needed and to place them instead in an Open Document spreadsheet.  There is an example already in trunk (doc/lang/hebrew.ods) for the Hebrew language.

By: Tilghman Lesher (tilghman) 2008-07-07 18:10:30

This patch is also not in compliance with the coding guidelines.  This needs to be fixed before the patch can be considered for commitment.  Specifically:

1) The spacing around if statements and opening braces is incorrect.  You are also not using braces on every conditional, as is now part of the guidelines.

2) You are using strsep to parse arguments, where you should be using the standard argument parsing macros.  Also, you are using the wrong argument delimiters for trunk (this would not be an issue if you used the standard parsing macros).

3) Your applications should be placed into their own source file, not within main/pbx.c.  Also, I'm not sure why you're mapping these API from say.c as function pointers.  As there are no competing implementations, you do not need these API calls to be function pointers.

4) Please do not call your function "universal", as it is not descriptive.  If you want to use vm_intro_ru for both Russian and Ukrainian, that's fine.  Also, calling the default function "counted_noun_en" is not acceptable, as people will expect that it should correctly pluralize terms in English.  Instead, don't create such a function at all, but merely omit appending anything (e.g. ending = "").  Ditto for "counted_adjective_en".

5) You're using tiny static buffers all over this patch for values for which you don't control the length.  It would be better to alloca() the correct length, when necessary.

By: David Chappell (chappell) 2008-07-23 11:45:29

I am working on a new version of this patch based on your comments.  I think I have the spacing problems fixed, I have moved the applications into their own file.  I still have to figure out the currently practice for parsing.

As for point 4, I think there is a misunderstanding due to the fact that the current patch enables this code only for Russian.  Since my new voicemail intro function works in a much more language neutral way, there is no reason why it could not replace the default function.  The next version of my patch will make this clearer.  Furthur, counted_noun_en() does in fact correctly pluralize English nouns within the scope of this problem.  Appending nothing would break it.  Again, this will be clearer in the next version of the patch.

By: Tilghman Lesher (tilghman) 2008-09-11 18:03:19

chappell: have you finished a new patch yet?

By: David Chappell (chappell) 2008-11-18 14:22:21.000-0600

I am working on this patch again.  The next version will be ready soon.

By: David Chappell (chappell) 2008-11-21 11:20:19.000-0600

I have uploaded vm_multilang.patch and app_say_counted.c.  I believe it answers all of Corydon76's comments.  The changes are:

1) spacing and bracket use changed to conform to guidelines
2) argument parsing macros are now used
3) applications moved into own source file
4) Changed name from "universal" to "multilang" and enabled for Russian, Ukrainian, and English to demonstrate that it truly supports multiple languages. (Though, based on an examination of the code for other langauges, I believe it can replace _all_ of the vm_intro_* functions, I enabled it only for languages that I know.)  Added extensive comments which, among other things, explain why the code to pluralize English nouns should be considered correct.
5) Replaced static buffers with alloca() buffers.

Enabling this code for a new languages requires these steps:
1) Add a rule to call vm_intro_multilang() from vm_intro().  This rule should indicate the gender for the word meaning "message"
2) Select the proper delension rule in say.c
3) Rename VM sound files to use standard naming convention (such as digit/1n rather than digit/odno)

By: Tilghman Lesher (tilghman) 2009-01-16 12:38:53.000-0600

Okay, this patch is mostly fine, and I will commit it here in a few minutes.  There is one more thing I would like to have from you, as a function of these changes.  I'd like you to submit a script spreadsheet for the Russian and Ukrainian languages (preferably in Open Office, though if you submit it in Excel, we'll do the necessary conversions).  Please follow the format established for the Hebrew language.  The file is located in doc/lang/hebrew.ods and contains three columns:  section of the code which requires it (core, app_voicemail, etc.); the filename, including any relative path (such as "digits/1"), and the script for a native speaker to follow, if they wished to record their own set of sounds.  Necessarily, the script should not be in a Western alphabet, unless the native language uses a Western alphabet.

By: Digium Subversion (svnbot) 2009-01-16 12:41:34.000-0600

Repository: asterisk
Revision: 168828

U   branches/1.4/apps/app_voicemail.c
U   branches/1.4/include/asterisk/say.h
U   branches/1.4/main/say.c

------------------------------------------------------------------------
r168828 | tilghman | 2009-01-16 12:41:34 -0600 (Fri, 16 Jan 2009) | 6 lines

Fix the conjugation of Russian and Ukrainian languages.
(related to issue ASTERISK-11877)
Reported by: chappell
Patches:
      vm_multilang.patch uploaded by chappell (license 8)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=168828

By: Digium Subversion (svnbot) 2009-01-16 12:49:08.000-0600

Repository: asterisk
Revision: 168832

_U  trunk/
U   trunk/apps/app_voicemail.c
U   trunk/include/asterisk/say.h
U   trunk/main/say.c

------------------------------------------------------------------------
r168832 | tilghman | 2009-01-16 12:49:08 -0600 (Fri, 16 Jan 2009) | 13 lines

Merged revisions 168828 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r168828 | tilghman | 2009-01-16 12:41:35 -0600 (Fri, 16 Jan 2009) | 6 lines
 
 Fix the conjugation of Russian and Ukrainian languages.
 (related to issue ASTERISK-11877)
  Reported by: chappell
  Patches:
        vm_multilang.patch uploaded by chappell (license 8)
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=168832

By: Digium Subversion (svnbot) 2009-01-16 12:53:47.000-0600

Repository: asterisk
Revision: 168835

_U  branches/1.6.0/
U   branches/1.6.0/apps/app_voicemail.c
U   branches/1.6.0/include/asterisk/say.h
U   branches/1.6.0/main/say.c

------------------------------------------------------------------------
r168835 | tilghman | 2009-01-16 12:53:46 -0600 (Fri, 16 Jan 2009) | 20 lines

Merged revisions 168832 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r168832 | tilghman | 2009-01-16 12:49:09 -0600 (Fri, 16 Jan 2009) | 13 lines
 
 Merged revisions 168828 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r168828 | tilghman | 2009-01-16 12:41:35 -0600 (Fri, 16 Jan 2009) | 6 lines
   
   Fix the conjugation of Russian and Ukrainian languages.
   (related to issue ASTERISK-11877)
    Reported by: chappell
    Patches:
          vm_multilang.patch uploaded by chappell (license 8)
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=168835

By: Digium Subversion (svnbot) 2009-01-16 12:55:12.000-0600

Repository: asterisk
Revision: 168836

_U  branches/1.6.1/
U   branches/1.6.1/apps/app_voicemail.c
U   branches/1.6.1/include/asterisk/say.h
U   branches/1.6.1/main/say.c

------------------------------------------------------------------------
r168836 | tilghman | 2009-01-16 12:55:12 -0600 (Fri, 16 Jan 2009) | 20 lines

Merged revisions 168832 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r168832 | tilghman | 2009-01-16 12:49:09 -0600 (Fri, 16 Jan 2009) | 13 lines
 
 Merged revisions 168828 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r168828 | tilghman | 2009-01-16 12:41:35 -0600 (Fri, 16 Jan 2009) | 6 lines
   
   Fix the conjugation of Russian and Ukrainian languages.
   (related to issue ASTERISK-11877)
    Reported by: chappell
    Patches:
          vm_multilang.patch uploaded by chappell (license 8)
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=168836

By: Tilghman Lesher (tilghman) 2009-01-21 12:25:22.000-0600

I'm going to go ahead and close this one out, but I'd still like to have the spreadsheet noted in the last bugnote.  Feel free to open a new issue with that.