This patch switches the radio buttons with checkboxes and replaces the global defines with TRUE and FALSE.

CommentFileSizeAuthor
bool.patch8.79 KBrobloach

Comments

nicholasthompson’s picture

nicholasthompson
Primary language English
commented

Hey rob. Thanks for this.

Question... What does this achieve?

Globalredirect used to use static Boolean values but I switched to defines for 'readability'... Do people believe TRUE/FALSE is better for checkboxes (or all settings?)

Also, if this were committed would we need an update hook to sort out/clean up the settings?

I would test it or look it up myself but I'm in a pub right now ;-)

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

This actually makes a lot of sense. We typically don't like to use defines for simple on/off settings, which most of these are. To core developers, I bet:
if ($alias && variable_get('globalredirect_case_sensitive_urls', 1)) {

is easier to read than:
if ($alias && variable_get('globalredirect_case_sensitive_urls', GLOBALREDIRECT_CASE_SENSITIVE_URLS_ENABLED) == GLOBALREDIRECT_CASE_SENSITIVE_URLS_ENABLED) {

nicholasthompson’s picture

nicholasthompson
Primary language English
commented
Status: Needs review » Closed (duplicate)

Fair point - I've gone off the define's too now ;)

I'm marking this as duplicate of a more recent post which has a more thorough refactoring patch in it... #803830: A bit of refactoring + mixed language redirects (only to keep the issue queue tidy)