In documentation we have this description:
$cfg['DefaultCharset'] string
Default character set to use for recoding of MySQL queries. This must be enabled and it's described by $cfg['AllowAnywhereRecoding'] option. You can give here any character set which is in $cfg'AvailableCharsets'] array and this is just default choice, user can select any of them.
As I unerstood, this directive is used on both pages Import and Export, to highlight default encoding charset option. But it's not using at all. On Export page we have another directive that makes exactly the same - $cfg['Export']['charset']. Strange logic, think it should be clear up. If we use $cfg['Export']['charset'] - let's make $cfg['Import']['charset'] that will take place on Import page. Or we have to use $cfg['DefaultCharset'] on Import and Export pages? May be this directives is useless and it would be better to delete them at all?
There is a little mess in code that combine options from array. Below is my suggestions for this part of code.
libraries/display_export.lib.php, line 206.
if ($cfg['AllowAnywhereRecoding'] && $allow_recoding) {
echo ' <label for="select_charset_of_file">'
. $strCharsetOfFile . '</label>' . "\n";
echo ' <select id="select_charset_of_file" name="charset_of_file" size="1">' . "\n";
foreach ($cfg['AvailableCharsets'] as $temp_charset) {
echo ' <option value="' . $temp_charset . '"';
if ((empty($cfg['Export']['charset']) && $temp_charset == $charset)
|| $temp_charset == $cfg['Export']['charset']) {
echo ' selected="selected"';
}
echo '>' . $temp_charset . '</option>' . "\n";
} // end foreach
echo ' </select>';
} // end if
libraries/display_import.lib.php, line 85.
if ($cfg['AllowAnywhereRecoding'] && $allow_recoding) {
echo '<label for="charset_of_file">' . $strCharsetOfFile . '</label>' . "\n";
echo ' <select id="charset_of_file" name="charset_of_file" size="1">' . "\n";
foreach ($cfg['AvailableCharsets'] as $temp_charset) {
echo ' <option value="' . htmlentities($temp_charset) . '"';
/* As possible variant.
if ((empty($cfg['Import']['charset']) && $temp_charset == $charset)
|| $temp_charset == $cfg['Import']['charset']) {
echo ' selected="selected"';
}
*/
if ($temp_charset == $charset) {
echo ' selected="selected"';
}
echo '>' . htmlentities($temp_charset) . '</option>' . "\n";
}
echo ' </select><br />' . "\n" . ' ';
} else {
echo '<label for="charset_of_file">' . $strCharsetOfFile . '</label>' . "\n";
echo PMA_generateCharsetDropdownBox(PMA_CSDROPDOWN_CHARSET, 'charset_of_file', 'charset_of_file', 'utf8', FALSE);
} // end if (recoding)
Logged In: YES
user_id=210714
Originator: NO
Thanks Victor,
Actually, $cfg['DefaultCharset'] only applies to the first paragraph of the explanation for $cfg['AllowAnywhereRecoding'], not to the second. I fixed the doc to add reference to $cfg['Export']['charset'].
Currently $cfg['Import']['charset'] does not exist.
About code suggestions (part 1), it's safer to use reset() but I agree that we don't need $temp_charset = reset... and we don't need the $key. I also agree that we can remove formatting (spaces) and I also removed newlines (anyway, firebug does a good job when showing source).
About code suggestions (part 2), I merged most of your suggestions (not the variant) but I kept the reset().
Logged In: YES
user_id=1686741
Originator: YES
Marc, thank you very much for such a detailed report. Now I see that $cfg['DefaultCharset'] is not for highlighting selected encoding option on Import/Export pages. Your job that has been done on documentation and Import/Export pages is fine and no comments here.
Only one thing: $cfg['Import']['charset'] is not a choice now, but it's logically come from $cfg['Export']['charset'], don't you think so?
Logged In: YES
user_id=210714
Originator: NO
Victor,
I agree but please open a feature request about this. Unfortunately we are drowning in feature requests but a patch would be appreciated ;)
Request is done. Think this one can be closed now.
https://sourceforge.net/tracker/index.php?func=detail&aid=2100910&group_id=23067&atid=377411