Has there been talk of implementing a drush command for this? Would that makes sense?

Just doing some one-off scripts, and would be interested in taking the time figure out how to write drush commands.

Anyone have any insight into how drush dl determines which modules get placed elsewhere on the system? For instance drush and drush_make used to go right to /usr/share or whatever. Not sure if that's hardcoded into drush or what.

CommentFileSizeAuthor
#17 drupalcs_drush-1547782-6060608.patch9.25 KBfluxsauce
#12 drupalcs_drush-1547782-5933316.patch8.74 KBfluxsauce
#5 drupalcs_drush-1547782.patch8.11 KBfluxsauce

Comments

patcon’s picture

patcon commented

FluxSauce and I are playing around with using RobLoach's Composer module to install the PHP_CodeSniffer dep and run it directly using Composer's autoloader functionality.

Not much there yet, as I'm just figuring out Composer as well as Drush's API, but it's here for anyone interested:
https://github.com/myplanetdigital/drupalcs

The idea would be to have drush dl drupalcs download the "module" (hopefully right to ~/.drush/drupalcs, as Composer module does). The rest of the install would involve running drush dl composer to download that module, and then running drush code-sniff. The first invocation would ideally run composer (which downloads the composer.phar file to ~/.drush/cache for all subsequent use), and then run drush composer install in ~/.drush/drupalcs. This resolves and fetches all the dependencies based on the composer.json file that I've got in my module repo, and drops then into a ~/.drush/drupalcs/vendor/ dir and builds an autoloader so that we can work with it in our drupalcs.drush.inc command file. Phewf!

I might have some of this stuff wrong, but I'll try to get Rob in here for feedback :)

das-peter’s picture

das-peter commented

This sounds indeed nice. Everything that makes using drupalcs more convenient is nice, it hopefully improves the usage of it and thus spreads the coding standard ;)
Unfortunately I'm not familiar with the composer module and can't really help.

patcon’s picture

patcon commented

Thanks @das-peter! I actually just came across your issue in the phpcs github repo, and the fact that it wasn't yet in a release was what got me going down this road into learning a bit about composer. Composer can use the repo at a given commit, rather than using only official releases :)

In case anyone's wondering, composer is an outside project that aims to be like the Bundler package manager in the Ruby world, used for handling rubygems management. PHP hasn't had a good, decentralized package manager like that. It's pretty much like drush_make for php libraries :)

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Thinking about this: as you can run drupalcs from the command line anyway, what's the point of having a drush integration?

fluxsauce’s picture

fluxsauce commented
Assigned: Unassigned » fluxsauce
Status: Active » Needs review
Issue tags: +drush
StatusFileSize
new drupalcs_drush-1547782.patch8.11 KB

Thanks for the introduction, patcon. I focused on the drush integration first; I haven't written any code for composer yet.

Thinking about this: as you can run drupalcs from the command line anyway, what's the point of having a drush integration?

  • Ease of use.
  • Familiarity of drush, arguably an industry standard.
  • Standardized defaults out of the box.
  • Command shortcuts without having to mess around with aliases.
  • Knowledge of relative site structure leveraging drush.
  • Improved installation.

Speaking to the improved installation; this is on two levels.

First, the symbolic link on installation is a workaround for a known limitation in PHP_CodeSniffer 1.3.3 and all previous versions; you can't specify the path of the Standard and have custom Sniffs work too. This is now fixed on the github repository, but not in PEAR. Therefore, the drush command I wrote checks to see if the Standard is installed (if you used the symbolic link), or if it isn't, if you have a version above 1.3.3 (does not exist yet) or a version from github. Depending on how it passes, drush will intelligently build the command.

The second level is integration with composer, which I have not written at this time, that will make it extremely easy to get drupalcs/PHP_CodeSniffer up and running, which will increase usability and improve user experience, which will in turn should lead to a greater adoption rate.

In short, this drush command will make it easier to use the Drupal PHP_CodeSniffer standards.

drush --help drupalcs
Executes PHP_CodeSniffer with Drupal Coding Standards on a particular directory.

Arguments:
 path                                      The path that you wish to scan. 


Options:
 --extensions=<php,module>                 Comma delimited list of file extensions to scan.    
                                           The default is                                      
                                           php,module,inc,install,test,profile,theme           
 --report=<xml>                            Specify the report format.  Options are: full, xml, 
                                           checkstyle, csv, emacs, source, summary, svnblame,  
                                           gitblame or hgblame.                                
 --report-file=</var/log/snifflog.xml>     Write the report to the specified file path.        


Aliases: dcs

drush drupalcs sites/all/modules/contrib/drupalcs/Test/

FILE: ...a/sf_sandbox/target/sites/all/modules/contrib/drupalcs/Test/bad.install
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
 13 | ERROR | Do not use t() in hook_schema(), this will only generate overhead
    |       | for translators
 16 | ERROR | Do not use t() in hook_schema(), this will only generate overhead
    |       | for translators
 53 | ERROR | Do not use t() or st() in installation phase hooks, use $t =
    |       | get_t() to retrieve the appropriate localization function name
--------------------------------------------------------------------------------


FILE: ...ia/sf_sandbox/target/sites/all/modules/contrib/drupalcs/Test/bad.module
--------------------------------------------------------------------------------
FOUND 6 ERROR(S) AFFECTING 6 LINE(S)
--------------------------------------------------------------------------------
 10 | ERROR | bad_install() is an installation hook and must be declared in an
...

I have attached a working patch, which includes the module definition (required for drush integration), the drush command, and an amendment to the README.

Feedback is welcome.

patcon’s picture

patcon commented

Looks awesome Jon! Full disclosure on the double-team: I work with Jon :)

I'm a little biased, but I'm in agreement. Coder, while a cherished baby of the Drupal community, separates us from the larger PHP community's tool. Coder is dead simple to get working with the tool we all use, so its use makes sense.

As painless as possible that we can make the transition, the better, in my opinions -- similar flags, similar defaults, similar output, similar install method, etc.

Right now, coder install is as easy as:

drush dl coder
drush coder-review [options] [severity] [review] [what]

phpcs is a little more involving. Really exciting for using composer to avoid system-wide pear access :)

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Why does this have to be a module? Does that mean that I have to download and install drupalcs for each separate drupal installation I have? Is there no way to install drupal independent drush commands?

patcon’s picture

patcon commented

I believe it could be just an extra file, drupalcs.drush.inc. It looks like fluxsauce has chosen to use a .module file, but I believe we could just as easily cut that down to one file. There might be another reason to do so:

If I understand correctly, drush5.1 downloads modules without .info and .module files straight to ~/.drush/[modulename] by default when using drush dl. (See composer module repo.) Not sure how it would treat the drupalcs project that also has a "Drupal/" directory (and "composer.json"), but I'll check it out. We might be able to convince the drush maintainers to change the behavior if it won't download to ~/.drush already.

So as I'm imagining it, drush dl drupalcs would ideally download the project to ~/.drush/drupalcs, dropping the Drupal standard, the composer.json and the drupalcs.drush.inc into place. We could add the logic on first execution of drush drupalcs to:

1) check for the availability of the phpcs command. If available use it, and if not:
2) check for the drush composer command (which would be in ~/.drush/composer) and if available, use that to download the components in composer.json to ~/.drush/drupalcs/vendor. If not available, then
3) throw an error telling the use to either install codesniffer via pear or install composer using "drush dl composer"

Would that work for you guys?

patcon’s picture

patcon commented

Yeah, here's the bit where drush checks:
http://drupalcode.org/project/drush.git/blob/refs/tags/7.x-5.1:/commands...

Seems if there are *.drush.inc files but not *.module files, it downloads to ~/.drush/

fluxsauce’s picture

fluxsauce commented
Status: Needs review » Needs work

Thanks for the feedback, klausi and patcon.

I'll take a different approach to make it a more proper drush extension, rather than using the module approach (it made sense at one point, but in the context of multiple site installations and command availability...).

I'll look into the composer integration; patcon can you submit a PHP_CodeSniffer package to http://packagist.org/ ?

patcon’s picture

patcon commented

Coolio. Although we can add a package to packagist, that will just be something we need to keep up to date -- plus, the one we want isn't even a full release, so we'd be putting up a package for a random pre-release commit hash.

We can just use the git repo directly, and it will be easy to update the reference from a commit to a tag when a new version is released and tagged:
https://github.com/myplanetdigital/drupalcs/blob/drush-command-composer/...

Hopefully the maintainer will cave and add a composer.json file soon enough :)

fluxsauce’s picture

fluxsauce commented
Status: Needs work » Needs review
StatusFileSize
new drupalcs_drush-1547782-5933316.patch8.74 KB

Here's the updated version. Major differences:

  • Does not need to be installed as a module (and in fact, can't, which is how it was originally)
  • README updated installation instructions
  • README clarification about independence from drush (you don't need drush to use drupalcs, but if you do it integrates nicely)
  • If Drupal standard not installed, use folder that drush command is in.

I'm considering composer integration as a separate feature for the time being. Let's focus on the initial integration first.

Feedback on the drush integration?

Darko’s picture

Darko commented

Installed and tested. It works as described.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work 
+++ b/README.txt
@@ -17,14 +17,43 @@ Requirements:
+
+While drupalcs can be used as a standalone set of rules for PHP_CodeSniffer, drush command support is included to facilitate ease of use, installation, and leveraging of drush features such as site aliasing.
+

Make sure that the lines wrap at 80 characters in README.txt

+++ b/README.txt
@@ -17,14 +17,43 @@ Requirements:
+wget http://ftp.drupal.org/files/projects/drupalcs-7.x-1.0-alpha2.zip

do not hardcode the current release in the README, so that we don't have to update it all the time.

+++ b/drupalcs.drush.inc
@@ -0,0 +1,242 @@
+function _drupalcs_extensions_default() {

While this is nice to check PHP files, drupalcs is also capable of checking CSS, JS, TXT and info files. Maybe there should be an option to run it on those, too. Maybe a "--mode" option with two possible values "default" and "extended", which is ignored if --extensions is present.

Command drupalcs needs a higher bootstrap level to run - you will need invoke drush from a more functional Drupal environment to run this command.
Why? drupalcs does not need Drupal?

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

There is now a dedicated installation doc page: http://drupal.org/node/1419988
So I think we should keep the additions to README.txt short, just mention that drush integration is available and link to the install page where we can list the instructions.

fluxsauce’s picture

fluxsauce commented

Just pinging back, going to work on this Friday. Thanks for the feedback & suggestions, @klausi

fluxsauce’s picture

fluxsauce commented
Version: 7.x-1.0-alpha1 » 7.x-1.x-dev
Status: Needs work » Needs review
StatusFileSize
new drupalcs_drush-1547782-6060608.patch9.25 KB

Major changes:

  • Added "mode" option as a shortcut to a group of file extensions.
  • Relaxed drush bootstrap to avoid dependency on Drupal installation.
  • Simplified installation instructions.
  • Static caching of command results (faster).

Documentation for installation page:

Installation: Drush

While drupalcs can be used as a standalone set of rules for PHP_CodeSniffer, drush command support is included to facilitate ease of use, installation, and leveraging of drush features such as site aliasing.

Extract the contents of the drupalcs project into one of the locations specified in the drush README.txt COMMANDS section, including:

  • In a subdirectory of the .drush folder in your home directory, like ~/.drush/drupalcs (you may have to create the .drush folder yourself)
  • In a folder specified with the --include option (see drush topic docs-configuration)
  • In /path/to/drush/commands (not recommended)

Example:

mkdir ~/.drush
cd ~/.drush
# download latest copy of drupalcs
unzip drupalcs-*.zip
rm drupalcs-*.zip

For more information about drush, see http://drupal.org/project/drush

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Fixed

Very good, committed! http://drupalcode.org/project/drupalcs.git/commit/8878df3

Would you be so kind to copy the text from above to the documentation page?

ericduran’s picture

ericduran commented

It should be noted, it we remove the .info file Drush will automatically download this to it's .drush directory.

patcon’s picture

patcon commented

@fluxsauce So awesome Jon! Nice man. Just... nice. :)

Really glad that this got committed. Thanks klausi!

fluxsauce’s picture

fluxsauce commented

Excellent, thanks all for your feedback and reviews! I've got a big grin on my face IRL.

Instructions updated - http://drupal.org/node/1419988

patcon’s picture

patcon commented
Issue tags: +Composer

tagging

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