I do not see any reason why the cache directory is a user setting (defaults to "files/xmlsitemap"). I see no good reason why users should change this folder. It's better to let the module manage it's directory themself. We know that files/xmlsitemap is a path that is only used by xmlsitemap. It's a standard rule to use the modules name as folder name here.

Having this in the user UI confuses users only. Options without a real use case shouldn't be in a good UI to reduce complexity whenever possible.

Comments

dave reid’s picture

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

I agree with you hass that this is definitely an 'advanced' option. Looking at core as the example I guess the only time you can configure a sub-files directory is the user pictures directory. But it looks like all other files directories are not configurable (css, js, color, languages, simpletest, imports, etc). I've kept the option exposed in 6.x-2.x for now, but I've been debating moving it to a collapsed-at-the-bottom 'Advanced settings' fieldset with other options like chunk size, number of links to process each batch, and configurable base url.

On the other hand, if it's not configurable, it saves a teeny bit code in 6.x-2.x since I take the time to validate the directory and if its changed, delete all the sitemap cache files and remove the old directory.

I think overall, my vote would to be to move it to a collapsed 'Advanced settings' fieldset.

[Edit: Not sure why I thought earnie made the initial post, but I changed it to hass. Sorry hass!]

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Postponed
Issue tags: +release 6.x-1.1

I agree but I'm postponing for later.

Anonymous’s picture

Anonymous (not verified) commented
Assigned: Unassigned »
Anonymous’s picture

Anonymous (not verified) commented
Status: Postponed » Active

I will implement this change by adding a [#disabled] = TRUE to the form item. This allows the user to see what the directory is but doesn't allow the user to change it.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Fixed

Committed to 6.x-1.x CVS.
http://drupal.org/cvs?commit=259504

Status: Fixed » Closed (fixed)

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

sebasto’s picture

sebasto commented

Well, I'm not sure it's a good solution (#4). I had a problem with multisites which could have been solved by setting the cache directory, and I found this #disabled to be very confusing.

Imho, it would be better to hide it if you want to keep it in your form but don't want users to change it ...

Anonymous’s picture

Anonymous (not verified) commented

@sebasto: The fix was committed to CVS and is in the -dev version. We are not coding to 1.x version any longer and this issue has been closed since September 2009. I suggest you move to the 2.x series, it is the one supported for Drupal 7.