kharbat [kharbat]

typeface.js is not released under GPL Licence; in that case, you cannot host it in the CVS repository. The fact the file is available for download from a third-party site is a reason more to not include it in Drupal.org.

The code should use Drupal functions, whenever possible; the code should use file_scan_directory(), not scandir().

There is still something to fix.

• typeface_fonts() still uses scandir.
• typeface_version() uses the variable $matches, but it's not a local variable, nor it's a global variable.
• In code like if(! function_exists('tf')) { there is no need to put a space after the !.
• In the module, comments declare some functions as custom hooks, but they are not technically hooks; to be hooks your module should contain code like module_invoke_all('fonts'), or module_invoke('fonts').Differently, they are simply functions.
• There is a function name that doesn't start with the name of the module (tf()).

This report can be set as closed/fixed by the person who approves your CVS application.

Regarding the last point, the function tf() extends function t() . it's easier to use and remember when it's short. I'm also checking for function definition to avoid any conflicts or errors.

tf() doesn't actually extend t() but it uses the function to return its output; I think it's a nuance.
Function names are prefixed with the module name to avoid conflicts; there are cases where it's acceptable that the function doesn't follow this rule, but they are limited cases. In any case, when the function name doesn't contain the module name as prefix, the function name is not two characters long, and it doesn't start with the name of a Drupal function.

The status of the report says that the code needs review from you.

I'm using the "needs review" for applicants to inform us, the CVS admins, a review is needed. I'm using "needs work" for the CVS admins to tell the applicants that after a review the applicant needs to do work.

Note, anyone can review. The CVS admins do review also but the status helps us admins track the process.

I guess I forgot once again to change the status; I apology for this.

 	if(!function_exists('typeface')) {
		function typeface($string, $font, $class = NULL, $css = NULL, $translate = TRUE, $args = array(), $langcode = NULL) {
			$string = ($translate ? t($string, $args, $langcode) : $string);
			return theme('typeface', $string, $font, $class, $css);
		}
	}

There is no need to check if typeface() is already defined, as the only module that can use such function is yours. I don't think typeface() is a PHP function (correct me if i am wrong).

It would also be better to use an array for the last parameters, and change the code accordingly.

function typeface_font_name($file) {
  $font_name = str_replace('.typeface.js', '', $file);
  $font_name = str_replace(array('-', '_', '.'), ' ', $font_name);
  $font_name = ucwords($font_name);
  return check_plain($font_name);
}

can be rewritten as

function typeface_font_name($file) {
  $replacements = array(
    '.typeface.js' => '',
    '-' => ' ',
    '_' => ' ',
    '.' => ' ',
  );
  return check_plain(ucwords(strtr($file, $replacements)));
}

Verify you replace all the tab indentations you used with two spaces indentation.

I am not sure that passing JSON data to PHP (which doesn't handle that type of data natively) is the simple way to do what you are trying to do.
You need a third-party JavaScript library because JavaScript doesn't natively support JSON (it is able to transform JSON data in native data using eval(), but it doesn't have a function to transform native data in JSON). I would try with a different format.

I would add the license for the JSON library you are using as a comment before the code. As the library is few code lines, I don't think it is a problem if it is include in one of your files. I am not sure its license is compatible with GPL License.

Effectively, the EULA is a license agreement; therefore, the license is reported.

Commercial Use. You may sell for profit and freely distribute scripts and/or compiled scripts that were created with the SOFTWARE PRODUCT.

Isn't this in contrast with the GPL License?

I am not changing the status of the issue, so other people can review it.

There are still some places where the code doesn't use Drupal functions (it uses ucfirst(), and strtolower()).

After this, I think the code could be accepted.

EDIT: There is a page in the Drupal API documentation that lists all the Unicode functions made available from Drupal. Of those functions, only few are not of any use for normal cases; the others can be normally used (and they should) by any modules.