[core] Implement GNU-style long options and '--version' by duanyang25 · Pull Request #3600 · pmd/pmd

duanyang25 · 2021-11-03T19:41:57Z

Describe the PR

This PR introduces GNU-style long options for existing parameters of PMD following Clément's suggestion on [core] Migrate CLI to using GNU-style long options #3424. However, I have not yet found a good way to "output a deprecation warning when the old variants are used", so I just keep all of the old variants unchanged. In this way, people who prefer long options can use these new options, and existing users can use PMD as usual. Additionally, I also added --fail-on-violation for CPD and kept --failOnViolation unchanged as what I did for PMD.
It also introduces the long option "--version" that behaviors as what [core] Add a --version CLI option #3425 mentioned, displaying the current version of PMD. I followed Clément's suggestion so that -version behaves as usual, and --use-version has the same behavior as -version, but "--version" follows the GNU-style and displays the current version of PMD.

I am an undergraduate student. One of the courses that I take this semester related to Software Engineering has assignments about fixing issues and hopefully getting merged. I have spent two weeks working on this issue alone, and I have added some unit tests and passed both them and existing tests when building it. I also tested them manually. Moreover, you are welcome to change the code that I committed. So please take a look at this PR. Thank you very much.

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)

… still figuring how to build successfully with existing unit tests

…ent the feature pmd#3424 and pmd#3425, and compile and build the project

oowekyala

Hi and thanks for your contribution!

This looks good overall, however, I think there's a misunderstanding about the meaning of the --use-version option. It's supposed to merge the functionality of -language and -version together, as having a pair of options is error prone. This is described in #3426

Let me know if you need help to tackle this remaing point.

--
I'll look into a way to produce a deprecation warning. We really need that I think, because -version and --version are not aliases for one another

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>

duanyang25 · 2021-11-06T18:40:58Z

Hi Clément,

Actually, this is my first PR for large projects. Thank you very much for reviewing my commits and providing suggestions. I just realize that I misunderstood the "--use-version", so I removed it and the corresponding unit tests I wrote before. Could you review the changes with new commits on this PR again, is it okay to be merged into some future versions?

Regarding the option "--use-version", I think it may be better to work on it separately. I will work on implementing it later with issue #3426 you mentioned and also the deprecation warning in a new PR if I have time.

ghost · 2021-11-06T19:54:40Z

	1 Message
📖	This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report

Generated by 🚫 Danger

oowekyala

Hi @duanyang25,

Actually, this is my first PR for large projects.

Kudos, this is a high-quality contribution :) You're welcome to work on #3426 separately, I agree that it's a better idea. This PR looks fine to me, I'll merge it for PMD 6.41.0. Thanks for taking this on!

Note that I've now implemented the part about emitting a deprecation warning

adangel · 2021-11-09T19:13:29Z

Thanks again for the PR!

Reminder: We also need to update the documentation at https://github.com/pmd/pmd/blob/master/docs/pages/pmd/userdocs/cli_reference.md and https://github.com/pmd/pmd/blob/master/docs/pages/pmd/userdocs/cpd/cpd.md
I think, it doesn't make sense to document the now deprecated flags, so the doc should only suggest the "new" way (either --long-option or single char -o options).

In https://github.com/pmd/pmd/blob/master/docs/pages/pmd/userdocs/installation.md there is once used -auxclasspath which should now be replaced with --aux-classpath.

This reverts commit a0410bd.

…p `-language` and `-version` unchanged

duanyang25 · 2021-11-10T14:52:27Z

Hi @oowekyala,

Thank you for implementing the deprecation warning, and thank adangel for posting these links. I have changed them accordingly, please take a look at my new commits for these markdown files.

Additionally, I put these long -- options before our old - options which will be deprecated in future large versions of PMD, so the output of --help is more straightforward to users.

adangel

Thanks for updating the documentation.
I'll try to add deprecation warnings to CPD as well (similar like Clément has added to PMD CLI).

adangel · 2021-11-11T08:25:32Z

@all-contributors add @duanyang25 for code

allcontributors · 2021-11-11T08:25:41Z

@adangel

I've put up a pull request to add @duanyang25! 🎉

[core] Implement GNU-style long options and '--version' pmd#3600 * pr-3600: (24 commits) [doc] Update release notes (pmd#3600, pmd#3424, pmd#3425) docs: update docs/pages/pmd/projectdocs/credits.md [skip ci] docs: update .all-contributorsrc [skip ci] [core] Add deprecation warnings for CPD CLI options Apply suggestions from code review [doc] CPD CLI: --filelist to --file-list remove the option introduced by a mistake Update cli_reference accordingly to adopt GNU-style long options, keep `-language` and `-version` unchanged move -- options before - options change CPD.md to the new option "--fail-on-violation" Revert "change it to the new option "--fail-on-violation"" change to be the first option on of CPD change it to the new option "--fail-on-violation" Change it to `--aux-classpath` Output a deprecation warning remove and will deprecate it later seperate and current work Update description of --version add additional unit tests regarding long options, successfully implement the feature pmd#3424 and pmd#3425, and compile and build the project fix one minor issue related to 'version' ...

duanyang25 added 6 commits November 3, 2021 08:04

Add long CLI options and 'version' options and keep old ones pmd#3424,…

e844dad

… still figuring how to build successfully with existing unit tests

follow code stdyle

f5bd670

follow code stdyle

59408d2

fix one minor issue related to 'version'

dfb83b3

fix one minor issue related to 'version'

3a8a950

add additional unit tests regarding long options, successfully implem…

e7d8b8a

…ent the feature pmd#3424 and pmd#3425, and compile and build the project

oowekyala self-requested a review November 3, 2021 22:40

oowekyala reviewed Nov 6, 2021

View reviewed changes

Comment thread pmd-java/src/test/java/net/sourceforge/pmd/cli/CLITest.java Outdated

Comment thread pmd-core/src/main/java/net/sourceforge/pmd/cli/PmdParametersParseResult.java

oowekyala reviewed Nov 6, 2021

View reviewed changes

Comment thread pmd-core/src/main/java/net/sourceforge/pmd/cli/PMDParameters.java Outdated

oowekyala reviewed Nov 6, 2021

View reviewed changes

Comment thread pmd-core/src/main/java/net/sourceforge/pmd/cli/PMDParameters.java Outdated

duanyang25 and others added 2 commits November 6, 2021 12:59

Update description of --version

5778c33

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>

seperate and current work

206b657

duanyang25 requested a review from oowekyala November 6, 2021 18:41

remove and will deprecate it later

4d52b26

oowekyala added this to the 6.41.0 milestone Nov 7, 2021

oowekyala self-assigned this Nov 7, 2021

oowekyala approved these changes Nov 7, 2021

View reviewed changes

oowekyala added 2 commits November 7, 2021 21:37

Output a deprecation warning

5ec56a9

Merge branch 'master' into pr/3600

2f787f7

This was linked to issues Nov 8, 2021

[core] Add a --version CLI option #3425

Closed

[core] Migrate CLI to using GNU-style long options #3424

Closed

duanyang25 and others added 6 commits November 10, 2021 07:06

Change it to --aux-classpath

dad5e0d

change it to the new option "--fail-on-violation"

a0410bd

change to be the first option on of CPD

33bfa1b

Merge branch 'master' of https://github.com/duanyang25/pmd

a944dd6

Revert "change it to the new option "--fail-on-violation""

4920482

This reverts commit a0410bd.

change CPD.md to the new option "--fail-on-violation"

04f4cdf

duanyang25 and others added 5 commits November 10, 2021 08:10

move -- options before - options

ea05792

Merge branch 'master' of https://github.com/duanyang25/pmd

0d90091

Update cli_reference accordingly to adopt GNU-style long options, kee…

a71ca23

…p `-language` and `-version` unchanged

remove the option introduced by a mistake

f83de43

Merge branch 'master' of https://github.com/duanyang25/pmd

bc7d4fb

duanyang25 requested a review from oowekyala November 10, 2021 14:53

[doc] CPD CLI: --filelist to --file-list

b342097

adangel approved these changes Nov 11, 2021

View reviewed changes

Apply suggestions from code review

65e3ef3

allcontributors Bot mentioned this pull request Nov 11, 2021

docs: add duanyang25 as a contributor for code #3606

Merged

[core] Add deprecation warnings for CPD CLI options

3749f3e

adangel changed the title ~~Implement GNU-style long options and '--version'~~ [core] Implement GNU-style long options and '--version' Nov 18, 2021

adangel assigned adangel and unassigned oowekyala Nov 25, 2021

adangel added a commit that referenced this pull request Nov 25, 2021

[doc] Update release notes (#3600, #3424, #3425)

6153d44

adangel merged commit d5df3bf into pmd:master Nov 25, 2021

adangel mentioned this pull request Aug 24, 2022

[core] Use PicoCli and unify PMD usage under a single main #4059

Merged

16 tasks

Uh oh!

Conversation

duanyang25 commented Nov 3, 2021 • edited by adangel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the PR

Related issues

Ready?

Uh oh!

oowekyala left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

duanyang25 commented Nov 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Nov 6, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oowekyala left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adangel commented Nov 9, 2021

Uh oh!

duanyang25 commented Nov 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adangel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adangel commented Nov 11, 2021

Uh oh!

allcontributors Bot commented Nov 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

duanyang25 commented Nov 3, 2021 •

edited by adangel

Loading

oowekyala left a comment •

edited

Loading

duanyang25 commented Nov 6, 2021 •

edited

Loading

ghost commented Nov 6, 2021 •

edited by ghost

Loading

oowekyala left a comment •

edited

Loading

duanyang25 commented Nov 10, 2021 •

edited

Loading