There are various functions in the base system (bootstrap.inc) using the language_*() namespace. Such as language_default(), language_list(), etc. There is however drupal_multilingual() that is not at all in this namespace and is therefore illogically placed in the API. I propose we rename this to language_multilingual() to get it under the same namespace as the other language functions. It is still located in the same place in bootstrap.inc as before.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Title: Naming cleanup: rename drupal_mutlilingual() to language_multilingual() » Naming cleanup: rename drupal_multilingual() to language_multilingual()

Typo in title.

Gábor Hojtsy’s picture

We can also go a step ahead and do language_count() as a function name, which would return the actual number. It is cast to bool when needed anyway, so it would work the same way in existing settings as drupal_multilingual() with the added benefit of being closer to the internal naming and giving more useful data. Feedback welcome.

Gábor Hojtsy’s picture

Title: Naming cleanup: rename drupal_multilingual() to language_multilingual() » Naming cleanup: rename drupal_multilingual() to language_multilingual() or language_count()

Change title accordingly.

plach’s picture

language_multiple() ?

drifter’s picture

Well, if language_count() returns 1, is that multilingual or monolingual? It's cast to TRUE. language_multilingual() seems to make more sense. If I truly need a count (why would I?) I probably need a list of languages as well, and that's a seperate function.

Gábor Hojtsy’s picture

Title: Naming cleanup: rename drupal_multilingual() to language_multilingual() or language_count() » Naming cleanup: rename drupal_multilingual() to language_multilingual()

@plach: maybe :)
@drifter: yeah, that is right... language_list() exists for that.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

This is RTBC.
I didn't try it out, but the only thing that could happen is a fatal, because some function call was overlooked. Since the tests pass, that is basically impossible.

plach’s picture

Honestly I don't find language_multilingual() very appealing, it's a bit redundant. And since many functions in the language_* namespace might end up becoming methods of a Language object, unless we define a static method, drupal_multilingual() might make more sense.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs review

@plach: I don't think its generally a good thing to have functionality specific code in the drupal_* namespace. language_defaul() is similarly global but its not drupal_language_default() or drupal_default_language() either. There are a lot of language_* functions loaded with bootstrap.inc, language.inc and locale.inc on pages (many of which will not be under the language class or other classes), so unifying this function under that umbrella would be good for DX IMHO.

I think the same applies to drupal_language_* functions. Its just a big mess if we have drupal_*() drupal_language_*() and language_*() functions for language stuff with drupal_*() and drupal_language_*() being the exceptions (1 function using drupal_*() and 2 functions using drupal_language_*()). We just established the language_*() namespace officially with language.module, even though some functions are in global files like bootstrap.inc and language.inc.

plach’s picture

@Gabor:

I think the same applies to drupal_language_* functions. Its just a big mess if we have drupal_*() drupal_language_*() and language_*() functions for language stuff with drupal_*() and drupal_language_*() being the exceptions (1 function using drupal_*() and 2 functions using drupal_language_*()).

Well, the idea behind drupal_language_types() and friends was that language_ prefixed functions were ones that should belong to a hypothetycal Language module, should we had one at that time, or belong to the Language subsystem. OTOH drupal_ prefixed functions were core, include/bootstrap level, functions, that somehow lived on a level even below the language system itself.

Anyway, I'm fine with renaming drupal_multilingual, I just don't find the current proposal a great improvement from the DX pov.

Gábor Hojtsy’s picture

@plach: language.inc and locale.inc are chock full of bootstrap level functions that live below the language system, such as ones deciding how to even instantiate $GLOBALS['language'] (which is in big part the root of the language system). If we'd go the way of using drupal_* for these, we'd need to rename language_default() and most functions in locale.inc, language.inc and other places working with languages or negotiation or whatnot to be under drupal_*(), and that is not really how the rest of global Drupal functions are named, not even those introduced in Drupal 7 for language. Also following your thinking, language.inc has functions like language_initialize(), language_url_split_prefix() or language_from_default() which are hardly explained as part of a language object are they? (Despite being directly in that namespace).

In short the overall aim is to unify language object/metadata related code under language_*() direct (instead of drupal_language_*(), drupal_*(), locale_language_*(), etc) and then figure out the namespace(s) for negotiation/providers/etc. as well. In that sense this issue is "paired with" #1387608: Unify language_list() and locale_language_list() currently (which moves onward with the locale_language_*() namespace eradication in favor of language_*()) and would be followed up with other issues later to keep cleaning up the namespace (such as getting rid of the only remaining piece in locale_language_*(), locale_language_name()).

This issue is just a small piece in that bigger picture.

Gábor Hojtsy’s picture

Another data point. It might be inferred from other functions that the drupal_* prefix is used because it is for global Drupal built-in stuff. Such as the difference between drupal_language_types() and language_types() I assume, where the former contains built-in types and the later returns the current configuration. (Not that I agree with that use of the namespace, it could be inferred based on functions like drupal_common_theme()). However, drupal_multilingual() is explicitly dependent on current configuration, it is not at all just a device to spit out built-in data.

sun’s picture

Title: Naming cleanup: rename drupal_multilingual() to language_multilingual() » Rename drupal_multilingual() to language_multilingual()
Status: Needs review » Reviewed & tested by the community
Issue tags: +API change

I think this makes sense.

A more clear issue summary wouldn't hurt though.

catch’s picture

Title: Rename drupal_multilingual() to language_multilingual() » Change notification for: Rename drupal_multilingual() to language_multilingual()
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active

This function mainly exists to save loading code / lists of languages if there's nothing to find. However once CMI comes in, I have a feeling the variable_get() is not going to be so cheap here, so we may end up finding out it's not a very good optimization in that context.

For now I'm fine with renaming this, not sure it's a net win, but it's not making things worse and cleans things out of the drupal_*() namespace a bit.

Needs a change notification.

Gábor Hojtsy’s picture

Title: Change notification for: Rename drupal_multilingual() to language_multilingual() » Rename drupal_multilingual() to language_multilingual()
Priority: Critical » Normal
Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Gábor Hojtsy’s picture

Issue tags: +language-base

Tagging for base language system.

Gábor Hojtsy’s picture

Issue summary: View changes

Fix typo in summary