Skip to content

[cli] Move CLI progressbar rendering to pmd-cli#4233

Merged
adangel merged 6 commits into
pmd:pmd/7.0.xfrom
jsotuyod:progressbar-revamp
Jan 16, 2023
Merged

[cli] Move CLI progressbar rendering to pmd-cli#4233
adangel merged 6 commits into
pmd:pmd/7.0.xfrom
jsotuyod:progressbar-revamp

Conversation

@jsotuyod

@jsotuyod jsotuyod commented Nov 25, 2022

Copy link
Copy Markdown
Member

Describe the PR

  • Took the chance to improve how we render it
  • Added support for interactive terminals
  • Improved the overall performance / thread-safety of report rendering to STDOUT

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@jsotuyod
[cli] Move CLI progressbar rendering to pmd-cli
b836a35 
 - Took the chance to improve how we render it
 - Added support for interactive terminals
 - Improved the overall performance / thread-safety of report rendering to STDOUT
@jsotuyod jsotuyod requested a review from oowekyala November 25, 2022 18:30
jsotuyod
jsotuyod commented Nov 25, 2022
View reviewed changes
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/reporting/GlobalAnalysisListener.java Outdated
jsotuyod
jsotuyod commented Nov 25, 2022
View reviewed changes
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java
jsotuyod
jsotuyod commented Nov 25, 2022
View reviewed changes
Comment thread pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/PmdCommand.java Outdated
@jsotuyod jsotuyod added this to the 7.0.0 milestone Nov 25, 2022
@ghost

ghost commented Nov 25, 2022 •
edited by ghost
Loading

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 25 violations,
introduces 15 new violations, 0 new errors and 0 new configuration errors,
removes 19 violations, 0 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 50508 violations,
introduces 34658 new violations, 9 new errors and 0 new configuration errors,
removes 138301 violations, 4 errors and 7 configuration errors.
Full report
✅ Compared to pmd/7.0.x:
This changeset changes 28 violations,
introduces 27 new violations, 0 new errors and 0 new configuration errors,
removes 24 violations, 0 errors and 0 configuration errors.
Full report
✅ Compared to master:
This changeset changes 50507 violations,
introduces 34666 new violations, 9 new errors and 0 new configuration errors,
removes 138302 violations, 4 errors and 7 configuration errors.
Full report
✅ Compared to pmd/7.0.x:
This changeset changes 25 violations,
introduces 22 new violations, 0 new errors and 0 new configuration errors,
removes 12 violations, 0 errors and 0 configuration errors.
Full report
✅ Compared to master:
This changeset changes 0 violations,
introduces 695517 new violations, 22 new errors and 0 new configuration errors,
removes 822667 violations, 17 errors and 7 configuration errors.
Full report

Generated by 🚫 Danger

@jsotuyod jsotuyod marked this pull request as ready for review November 26, 2022 18:46
jsotuyod
jsotuyod commented Nov 26, 2022
View reviewed changes
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/reporting/FileAnalysisListener.java
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/reporting/GlobalAnalysisListener.java
adangel
adangel reviewed Dec 1, 2022
View reviewed changes
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/cli/PMDParameters.java
@adangel

adangel commented Dec 1, 2022

Copy link
Copy Markdown
Member

Is #3913 also resolved with this PR? If I understand it correctly (please correct me otherwise): The progress bar will be automatically disabled when we report to stdout. It will stay enabled, when we report into a file (-r file). By default, the progress bar is enabled.

Hm... I think, #3913 is about when running PMD within a non-interactive terminal. With this PR, the progress bar will be displayed using \r mode, e.g. the would result in many many lines in a CI build log. So, I guess, we still need something like a --batch-mode as suggested in #3828, correct?

@jsotuyod

jsotuyod commented Dec 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Right now progress bar is ON by default. We may decide to change that, and either have it OFF by default, or on by default for interactive consoles only (both plausible). Either scenario would solve #3913, but in the meantime it is still an issue.

In any case, we don't need a new --batch-mode flag (we already have --no-progress to explicitly turn it off, regardless of defaults). #3828 would be resolved by this PR.

@adangel

adangel commented Dec 1, 2022

Copy link
Copy Markdown
Member

Thanks for clarifying! 👍

@jsotuyod jsotuyod added the in:cli Affects the PMD Command Line Interface label Dec 1, 2022
oowekyala
oowekyala reviewed Dec 4, 2022
View reviewed changes
Comment thread pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/PmdCommand.java Outdated
Comment thread pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/PmdCommand.java
@oowekyala oowekyala mentioned this pull request Dec 4, 2022
Open
@jsotuyod @oowekyala
Use shorthand method
729ce9c 
Co-authored-by: Clément Fournier <clement.fournier@tu-dresden.de>
@adangel adangel linked an issue Jan 13, 2023 that may be closed by this pull request
[core] Progress reporting #3828
Closed
@adangel adangel self-assigned this Jan 16, 2023
adangel added a commit that referenced this pull request Jan 16, 2023
@adangel
Merge pull request #4233 from Monits:progressbar-revamp
9bc6226 
[cli] Move CLI progressbar rendering to pmd-cli #4233
@adangel adangel merged commit f482bbb into pmd:pmd/7.0.x Jan 16, 2023
@adangel adangel mentioned this pull request Jan 16, 2023
Closed
@jsotuyod jsotuyod deleted the progressbar-revamp branch January 16, 2023 12:51
This was referenced Jan 16, 2023
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adangel adangel adangel left review comments

@oowekyala oowekyala oowekyala left review comments

Assignees

@adangel adangel

Labels

in:cli Affects the PMD Command Line Interface

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

[core] Progress reporting

3 participants

@jsotuyod @adangel @oowekyala