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

solve issue #501 #520

Merged
merged 1 commit into from
Aug 11, 2024
Merged

solve issue #501 #520

merged 1 commit into from
Aug 11, 2024

Conversation

HennyNile
Copy link
Contributor

@HennyNile HennyNile commented May 21, 2021

Fixes #501
Hi! #501 mentioned can not use @-syntax in conjunction with arguments starting with literal @. I solve this proplem by changing the judge rule of @-syntax. The initail version recognizes the parameter value starting with @ as a file when expandAtSign is true whcih makes the program fail when some patameters' value just begin with @ and is not a file name. Then I add a judge rule that paramter value is not a file name when it begin with @/ . This is because filename in most OS can not begin with /, then the parameter value begin with @/ must not be a filename.
This change bring some benefits but also add some complexity. If you want a parameter value begins with @ and not a file name like @password, you should transfer @/@password into.

@HennyNile HennyNile force-pushed the master branch 2 times, most recently from b7abc5b to b926897 Compare May 21, 2021 13:32
@mkarg
Copy link
Collaborator

mkarg commented Sep 1, 2022

Thank you for looking into this. Indeed I do not like the idea of having a slash after @. Maybe we can find a solution that works without?

@mkarg
Copy link
Collaborator

mkarg commented Apr 27, 2024

@HennyNile Your PR is in conflict with the target branch. Can you please resolve the conflict and answer the open question? Thanks.

@mkarg
Copy link
Collaborator

mkarg commented Jul 6, 2024

@HennyNile Kindly requesting your response!

@HennyNile
Copy link
Contributor Author

I am very sorry for ignoring the previous messages. I will take to solve this conflict and find some better solution in the coming week.

@mkarg
Copy link
Collaborator

mkarg commented Jul 6, 2024

No need to be sorry! :-)

@mkarg
Copy link
Collaborator

mkarg commented Jul 28, 2024

@HennyNile Any news? :-)

@HennyNile
Copy link
Contributor Author

Hi, I am currently working on something important and will be available after July. If it's okay, I will address this issue at that time.

@HennyNile
Copy link
Contributor Author

@mkarg Hi, I’m currently working on this issue and will discuss it with you once I’ve found a suitable solution.

@mkarg
Copy link
Collaborator

mkarg commented Aug 9, 2024

@mkarg Hi, I’m currently working on this issue and will discuss it with you once I’ve found a suitable solution.

Sounds great! :-)

@HennyNile HennyNile reopened this Aug 10, 2024
@HennyNile
Copy link
Contributor Author

@mkarg In retrospect of this problem, I found another way to solve it and don't change the initial usage of @ syntax.

While jcommander expands the @ syntax before it tries to parse the parameter values, it's hard to distinguish whether a string starting with @ is an @ syntax usage or a normal parameter value. Thus, I change to expand @ syntax during the parsing stage. If we meet a string starting with @ before we meet a parameter key, it is @ syntax usage. Otherwise, it is a normal parameter value.

Hope this solution helps.

@mkarg mkarg merged commit 59ccac9 into cbeust:master Aug 11, 2024
2 checks passed
@mkarg
Copy link
Collaborator

mkarg commented Aug 11, 2024

@HennyNile Thank you very much for your contribution! 👍 😄

@mkarg mkarg self-assigned this Aug 11, 2024
@mkarg mkarg added the bug Bug label Aug 11, 2024
@HennyNile
Copy link
Contributor Author

Happy to help!

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

Successfully merging this pull request may close these issues.

Cannot use @-syntax in conjunction with arguments starting with literal @
2 participants