Skip to content

Conversation

@arnested
Copy link
Owner

The use of dir-locals (dir-locals-set-class-variables' anddir-locals-set-directory-class' in `drupal-detect-drupal-version' has proved not to be an ideal solution.

The "project local" variables is not always set correctly resulting in some buffers not entering drupal-mode and/or not having drupal-rootdir set. This limits usabilty of drupal-mode a lot.

We have to redo the handling of the project local variables. And a cleanup of the detection and parsing of drupal-rootdir, drupal-version, etc. could also be a good idea.

@dhaley
Copy link

dhaley commented Aug 21, 2013

@arnested, What about using 'drush dd' as a fallback method as done here:

https://github.com/kostajh/subDrush/blob/f9d8ba0c9a9d7153a883329c9bec88679cda64f8/lib/drush.py#L180

@arnested
Copy link
Owner Author

I just added a rewrite that doesn't use dir-locals.

It is definitely not the fancy cleanup of drupal-detect-drupal-version I've been dreaming of - but getting rid of dir-locals is more important than never getting around to refactoring it the right way.

I'm going to try this branch for some days before merging and releasing.

@dhaley, I have considered using drush dd but since this could be called on a lot of file visits I think it would become very slow.

arnested added a commit that referenced this pull request Jan 28, 2014
@arnested
Copy link
Owner Author

I just pushed a newer version that fixes some issues with the new approach.

The current version also uses temporary buffers for finding versions so you won't have a lot of visited files around that you haven't visited yourself.

drupal-detect-drupal-version is also much cleaner to look at now.

I'll keep using this version some days before merging it.

@ghost ghost assigned arnested Jan 28, 2014
@arnested arnested closed this in 65efa5b Jan 29, 2014
@arnested arnested deleted the feature/no-dir-locals branch January 29, 2014 12:25
@arnested
Copy link
Owner Author

It didn't reveal any problems today - so I just merged it to develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants