Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Remove dash from illegal characters. #6300
Remove dash from illegal characters. #6300
Changes from 1 commit
2b7fad5
f332e42
38687aa
feb6709
23d9720
a90ac60
ed42ac1
6fc9645
00f8c92
e39cca3
471ec0e
19e0879
2161cd1
ae8b183
08257ff
d0e5241
8a510e3
d7d5ac7
dda3259
7362457
fe43020
aae2b0e
4807493
0b1ca1a
d64b911
6032766
995ff60
4d37195
3f5b2e5
6db1344
c1ec4ad
cfeea59
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why didn't you remove the
-
it here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the recommendation in the README, I thought it was a mistake to include a dash in illegal signs, not those not recommended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@koppor so should i remove dash from unwanted characters and make update on READEME file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove it from illegal, but keep it in unwanted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the usage of
KEY_UNWANTED_CHARACTERS
.The code comment reads as follows:
Thus, the
KEY_UNWANTED_CHARACTERS
clearly is a subset ofKEY_ILLEGAL_CHARACTERS
. Not sure, whetherKEY_ILLEGAL_CHARACTERS
makes sense at all. Maybe because of tool interopability.I see three options:
A) Remove
KEY_UNWANTED_CHARACTERS
completely - and also the preference for it.B) Set
KEY_UNWANTED_CHARACTERS
to a minimum - as indicated in the comment. I would propose{}"
since these are the characters causing issues at parsing. I would not add space since this did not cause any issues until now (at list that I am aware of)C) Just remove
-
fromKEY_UNWANTED_CHARACTERS
Please do option B). Please rename
KEY_UNWANTED_CHARACTERS
toKEY_CHARACTERS_CAUSING_PARSING_ERRORS
. Please add a comment above that this is a subset ofKEY_ILLEGAL_CHARACTERS
.