Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a list of files to be checked #83

Merged
merged 1 commit into from
Sep 25, 2020

Conversation

sammarshallou
Copy link
Contributor

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).

index.php Outdated Show resolved Hide resolved
index.php Outdated Show resolved Hide resolved
index.php Show resolved Hide resolved
@sammarshallou
Copy link
Contributor Author

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?

Copy link
Member

@stronk7 stronk7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks perfect, thanks!

@stronk7 stronk7 merged commit 32156ed into moodlehq:master Sep 25, 2020
@stronk7
Copy link
Member

stronk7 commented Sep 28, 2020

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...

stronk7 added a commit to stronk7/moodle-local_codechecker that referenced this pull request Sep 28, 2020
They were leading to the whole codebase being checked since moodlehq#83.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants