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

In MapSplitter, ignore key-value separator after first #2663

Open
wants to merge 1 commit into
base: master
from

Conversation

@jiafu1115
Copy link

commented Nov 30, 2016

It is one important issue need to be fixed to support most cases which values may contain the separator.

for one example:
I can make sure my key can't contain separator. but I can't make sure my value can't contain separator due to the value is generated by third-part service or special codec which can get separator. So I think we should fix this issue. It is common issue for most of case, Throwing IAE isn't reasonable or easier to let's happen low rating bug.

Can someone help to take a look at this PR. Thanks.@cgdecker

@googlebot

This comment has been minimized.

Copy link
Collaborator

commented Nov 30, 2016

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no label Nov 30, 2016

@jiafu1115

This comment has been minimized.

Copy link
Author

commented Nov 30, 2016

I signed it!

@googlebot

This comment has been minimized.

Copy link
Collaborator

commented Nov 30, 2016

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Nov 30, 2016

@jiafu1115 jiafu1115 force-pushed the jiafu1115:patch-1 branch from 00daa93 to ad58144 Nov 30, 2016

@jiafu1115 jiafu1115 force-pushed the jiafu1115:patch-1 branch from ad58144 to e68dcf1 Nov 30, 2016

@jiafu1115

This comment has been minimized.

Copy link
Author

commented Dec 6, 2016

anyone can take a look?

@liach

This comment has been minimized.

Copy link

commented Dec 7, 2016

@jiafu1115 Should this regard different types of splitters? Or add some method like ignoreExtraSeparator()?

@jiafu1115

This comment has been minimized.

Copy link
Author

commented Dec 8, 2016

@liach no. only change the inner behavior for MapSplitter. This is the simplest way to meet requirement.WDYT?Thanks

@liach

This comment has been minimized.

Copy link

commented Dec 8, 2016

There must be a reason why it throws an exception when detecting more than one separators. You should ping a project manager.

@jiafu1115

This comment has been minimized.

Copy link
Author

commented Dec 8, 2016

@liach I had pinged the issue creator @cgdecker who is also the member for guava but no feedback util now. So I does not know who I should ping. so just waiting now.

@liach

This comment has been minimized.

Copy link

commented Dec 8, 2016

I think we need to ping the authors of Splitter. @lowasser

@jiafu1115

This comment has been minimized.

Copy link
Author

commented Dec 8, 2016

@liach Thanks for your reminder. We may need to wait and wait and wait......

@eamonnmcmanus eamonnmcmanus changed the title Fix #1900 In MapSplitter, ignore key-value separator after first Dec 8, 2016

@jiafu1115

This comment has been minimized.

Copy link
Author

commented Dec 13, 2016

@eamonnmcmanus Thanks for your change for title, it is more clear now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.