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

Show 100% on last line of progress meter #1697

Closed
GaryJones opened this issue Oct 11, 2017 · 7 comments
Closed

Show 100% on last line of progress meter #1697

GaryJones opened this issue Oct 11, 2017 · 7 comments

Comments

@GaryJones
Copy link
Contributor

PHPCS progress meter

Only a minor UX improvement, but the last line of the progress meter may be clarified better by including the full number of files and percentage i.e. for the above example, 68/68 (100%).

@gsherwood
Copy link
Member

If I make this change, the output will end up like this:

$ phpcs
............................................................ 60 / 71 (85%)
......E.E..                                                  71 / 71 (100%)

For short runs, you'll get something like this:

$ phpcs
..E.. 5 / 5 (100%)

I'm not really sure if this is a good change, so I'm hoping to get more feedback.

@gsherwood
Copy link
Member

Git diff for this change if anyone wanted to try it:

$ git diff src/Runner.php
diff --git a/src/Runner.php b/src/Runner.php
index 75b8b1c7b..e9ce80ff4 100644
--- a/src/Runner.php
+++ b/src/Runner.php
@@ -810,6 +810,15 @@ class Runner
             echo str_repeat(' ', $padding);
             $percent = round(($numProcessed / $numFiles) * 100);
             echo " $numProcessed / $numFiles ($percent%)".PHP_EOL;
+        } else if ($numProcessed === $numFiles) {
+            $padding  = (strlen($numFiles) - strlen($numProcessed));
+            if ($numFiles > 60) {
+                $padding += (60 - ($numFiles - (floor($numFiles / 60) * 60)));
+            }
+
+            echo str_repeat(' ', $padding);
+            $percent = round(($numProcessed / $numFiles) * 100);
+            echo " $numProcessed / $numFiles ($percent%)".PHP_EOL;
         }

     }//end printProgress()

@jrfnl
Copy link
Contributor

jrfnl commented Oct 11, 2017

LGTM. It would also be more consistent with other industry standard tools like PHPUnit. I just came across their changelog entry from 2015 where they made the same change 😉

Refs:

@GaryJones
Copy link
Contributor Author

screenshot of progress meter with final line numbers

LGTM as well.

(Aside: The magic number 60 is now used several times, and could be moved into a descriptive variable.)

@gsherwood
Copy link
Member

The magic number 60 is now used several times, and could be moved into a descriptive variable.

I knew I shouldn't have posted rough code :) It's not what I would commit - just what I hacked up to see the result.

@gsherwood gsherwood added this to the 3.1.1 milestone Oct 12, 2017
@gsherwood
Copy link
Member

I found an issue when implementing this, which is that the progress report doesn't show skipped files. I'll fix that at the same time. It wasn't really a problem before, but now the progress needs to know that all files were processed to be able to print 100%.

gsherwood added a commit that referenced this issue Oct 12, 2017
…sing (request #1697)

Also fixed an issue where progress output wasn't showing skipped files. Skipped files now set their path correctly even though they are not tokenized, and are reported by the runner and not the file itself.
@gsherwood
Copy link
Member

This has been added now. Given it's also fixing the missing the display of skipped files, I've rolled it into the next bug fix release.

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

No branches or pull requests

3 participants