Christian Weiske [2005-12-04 20:08 UTC] Nice package I'd like to see in PEAR.
However, you should change the die() calls to returning error objects.
Arnaud Limbourg [2005-12-05 15:16 UTC] Apart from minor issues (I had to point it out :) there is one major problem with this package. GPL is the chosen license and PEAR does not allow GPL.
Since the package is based on GPL version of a Perl module I am not sure if there is anything we can do about it. Thoughts from others ?
Nicholas Pisarro [2005-12-05 16:12 UTC] My code is BSD. It's only the language data file that's GPL.
Stefano F. Rausch [2005-12-05 18:10 UTC] If I'm not mistaken, once you include a GPL "file", GPL overrides automatically the existing - in this case BSD - license. This is what Arnaud is hinting at.
I'm sorry to say that I'm of the same opinion - I would be glad to be corrected, for I very much like your proposal. I checked out a German and an Italien short sentence (lesser than 40 chars) and I was pleased ;)
Isn't there any other source you can use as a language reference?
Nicholas Pisarro [2005-12-05 20:45 UTC] I contacted the author of the perl package, who advised me that database data such as this is not subject to copyright.
Arnaud Limbourg [2005-12-05 21:38 UTC] Then you can use the data and if your package is under the BSD you can go on :)
Stefano F. Rausch [2005-12-05 22:11 UTC] That's good news!
If you clear up the code PCS-wise [1] - e.g. correct the file header and use spaces around the array key/value's operator etc., I'm with you.
[1] http://pear.php.net/manual/en/standards.php
bertrand Gugger [2005-12-07 19:56 UTC] Bravo, it's very nice done !
Just 2 points:
* perhaps is done and I missed it, sorry if it's the case, but shouldn't you compress all multiple spaces (original or derived from non alpha) in a single space ?
* the datafile should not be in the php directory, you should use the task/replace from the installer to have it in the data dir. (see Validate_BE and its package.xml for example and doc in http://pear.php.net/manual/en/guide.developers.package2.tasks.php#guide.developers.package2.tasks.replace )
Stefano F. Rausch [2005-12-13 20:29 UTC] Do just a few more PCS correction and you're almost on track:
- some docs comment, e.g. method _sort_func($a, $b), are missing the @return tag
- try to break the lines at approx. 80-85 chars, e.g. method _next_char(&$str, &$counter, $special_convert = false)
- convert all tabs to 4 spaces for the indentation
- examples/unit test: require_once is not a function, but a language construct. Do remove the paranthesis.
Even if it does annoy you, please adhere to the PCS.
I'm looking forward to voting!
anatoly techtonik [2005-12-18 19:36 UTC] русский язык не идентифицируется
Russian language is not identified.
BTW, how about encodings. In Far Manager if I can enter russian text in windows-1251 and in koi8-r and some others which are automatically autodetected. The correct encoding is more important than just "language". Language information has only sematic meaning, while quite often encoding is the primary object you need to know to operate.
Nicholas Pisarro [2005-12-18 23:53 UTC] The problem with russian text was web/http problem. I added a header to the example to fix it.
It would be nice to have another class to deal with character encodings, as well as one for the unicode standards
|
