moodlehq / moodle-local_codechecker

A Moodle local plugin providing a simple web UI to run the MoodleCS coding style checks with PHP_CodeSniffer.
63 stars 72 forks source link

Allow a list of files to be checked #83

Closed sammarshallou closed 4 years ago

sammarshallou commented 4 years ago

Here's a simple change that lets you paste in a list of files to the textarea. It's super useful if checking files manually, because you can just do 'git show --name-only' on your commit, and then paste in the list of files to check.

I also increased the time limit to 600 because checking is so slow nowadays (which actually, is why it's often more useful to check a list of files rather than a whole folder at once - although often you also want to check files from multiple folders).

sammarshallou commented 4 years ago

I've pushed a new version and changed the way it works a bit - now, it will output the failures [file not found] for each path (instead of stopping at the first path that fails). So you get multiple errors if necessary. If there are any failures, it doesn't run the checks. I tried this out, it seems to work OK.

Also added in the clean_param as pointed out.

I think this one is better?

stronk7 commented 4 years ago

Fun, just real usage experience... I was running some tests in the UI against mod/label... and had to kill apache a number of times... just to discover that, with the new text box, if you enter some blank line... the whole dirroot is checked (and it can take hours).

And it's really easy for that to happen (if you were used to the old input field and form being submitted on return). In fact I had 3 in my box, heh.

So we need to add something like this, to prevent blank lines to be computed as dirroot:

@@ -61,7 +72,10 @@ if ($pathlist) {
     $failed = false;
     $fullpaths = [];
     foreach ($paths as $path) {
-        $path = clean_param($path, PARAM_PATH);
+        $path = trim(clean_param($path, PARAM_PATH));
+        if (empty($path)) {
+            continue;
+        }
         $fullpath = $CFG->dirroot . '/' . trim($path, '/');
         if (!is_file($fullpath) && !is_dir($fullpath)) {
             echo $output->invald_path_message($path);

Will create a patch soon with that...