jittiang [jittiang]

1.     '#title' => t('Dictionary settings'),
       '#description' => t('You must Re-index site at Search settings after change Dictionary settings.')
   

   Strings used in user interface should have the first word in capital case, and the other words in lower case (with the exception of proper nouns, adjectives derived from proper nouns, and acronyms).

2.   $form['siam_wrapper']['test_saparate']['saparate_button'] = array(
       '#type' => 'submit',
       '#value' => t('Separate Now!'),
       '#description' => t("Separate Original Text"),
       '#weight' => 1,
       '#ahah' => array(
         'path' => 'admin/settings/siam/js',
         'wrapper' => 'saparate-text',
         'effect' => 'fade',
       ),
     );
   

   The field submit doesn't use the attribute #description.

3.   drupal_json(array('status' => TRUE, 'data' => $saparate_txt));
   

   Replace any occurrence of saparate with separate.

4.   switch ($form_state['values']['op']) {
       // …
     }
   

   There should not be the need to check that value (which could not be present), if the code would use two different submission functions (one for the form, and one for the button Remove.

5.     foreach ($tdict as $tdict_file) {
         if($tdict_file) {
           $file = new SplFileObject( $path . $tdict_file);
           while (!$file->eof()) {
             $word = $file->current();
             $word_len = mb_strlen($word);
             $word = mb_substr($word, 0, $word_len-1);
             $dict->add($word);
             $file->next();
           }
         }    
   

   Isn't SplFileObject a PHP 5 class?
   The code should use the Drupal Unicode functions.

6. See the Drupal coding standards to understand how a module code should be written. In particular, see how the code should be formatted, and how constants should be written.

7. function siam_uninstall() {
     drupal_uninstall_schema('siam');
     
     if(variable_get('dict_checkboxes_select', false)) {
       variable_del('dict_checkboxes_select');  
     }
     if(variable_get('dict_taxo_select', false)) {
       variable_del('dict_taxo_select');  
     }
     if(variable_get('dict_create_own', false)) {
       variable_del('dict_create_own');  
     }
     if(variable_get('dict_upload_checkboxes_select', false)) {
       variable_del('dict_upload_checkboxes_select');  
     }
     if(variable_get('dict_file_upload', false)) {
       variable_del('dict_file_upload');  
     }
   }
   

   The code should delete the Drupal variables whatever values they have.

In the coding standards there is also a part about the namespace respect, which reports the correct names to use to avoid conflict with other modules. What I reported about using Drupal Unicode functions is still true; those functions are still not used from the code.

I would rather change the name of the module. As it is now it is too generic; it would better to name it dict_siam.module, in example. Anyway, this is not something that block the module from being accepted.

    '#type' => 'fieldset',
    '#title' => t('Select dictionary'),
    '#description' => t('Dictionary from LibThai (http://sourceforge.net/projects/libthai/)'),

Files available from third-party sites should not be included in Drupal.org CVS. In Drupal.org CVS the only accepted files are the ones licensed under GPL license v2+; compatible licenses are not accepted (the dictionary files are licensed under GNU LGPL).

See the Drupal coding standards to understand how a module code should be written. In particular see the namespace respect; I have already reported this in comment #8, but still the code doesn't respect the namespace.

The code depends on PHP 5, but it doesn't declare its dependency from it.

I apologize for the delay in approving this application.