Qualifier values do not have first character converted to lowercase #64
Conversation
FromString panics for some inputs. This change fixes the issue and adds a test to ensure it works.
| // only the first character needs to be lowercase. Note that pURL is always UTF8, so we | ||
| // don't need to care about unicode here. | ||
| Value: strings.ToLower(value[:1]) + value[1:], | ||
| value = strings.ToLower(value[:1]) + value[1:] |
There was a problem hiding this comment.
I am not even sure the lowercase first character is needed. I can't see it specified in the spec.
There was a problem hiding this comment.
@tommyknows Hey Ramon, do you know if we really need lowercase characters here?
There was a problem hiding this comment.
Mh, I thought this was just the behaviour I ported from before the refactor, but I actually can't seem to find that 👀
i wonder why I've added it in that case...
Also, it's not in the spec and I can't see any test cases requiring this. I'd say remove it then...
There was a problem hiding this comment.
I am in favour of removing the lowercase conversion. I updated the title of the pull request and added another commit to just remove the case conversion.
wetterjames4
changed the title
This is not required by the spec or any test cases.
|
I think we can close this PR here, because the commits are already in: #65 |
FromString panics for some inputs. This change fixes the issue and adds a test to ensure it works.