-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix issue 1474 (to check invalid sheet name) #1484
Fix issue 1474 (to check invalid sheet name) #1484
Conversation
Hello! I suggest you take a look at this, do you think it would be good to also add these other checks? Edit: oops, the other checks already exist, I missed them, sorry about that. However I noticed your regex does not catch single quotes, was this intended? |
hi,papb,Your method is more concise, I think it's very good. About double quotation marksIt seems that the naming rules of sheet have something to do with the version of Excel As for single quotation marksAs far as I know(Just now), single quotation marks can be placed in the middle, but not the head and tail. Can you test the single quotation marks for me? |
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.
Just some minor feedback around the readability/maintainability of the test case, then we're good to go!
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.
thank you!
Although here has been a long long times since this change applied. |
Summary
Fix issue #1474 .
An exception is thrown when the name is illegal.
Test plan
For this I have added unit tests.