-
Notifications
You must be signed in to change notification settings - Fork 640
translate: support format option to specify html or text #1574
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
Conversation
|
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.
|
|
Do you think something like is-html would be reliable? |
|
You mean using is-html to test the input? That could work, but what if the input is not html but the caller wants html output, or the other way around? Having an option parameter override covers that case. Btw, I signed the CLA. |
|
CLAs look good, thanks! |
Are there reasons why someone would want to do that? I assume the API wouldn't do its job as well if the format didn't match the input. |
That is the current behaviour: The API auto detects the source language but doesn't auto detect the format type, it defaults to html if the format parameter is missing, regardless of the actual input format. So the caller can choose the output format. That is also how the Google Node.js client works as well, by passing a format parameter. Which is why I thought the a parameter should be used here too, as I was coming from that client. I suppose other devs coming from the node client would expect it to work the same way. |
|
I think this is a case where we can try to out-smart the API. If we can detect that the user's input is HTML or text, then they shouldn't need to override it. However, I'm not certain var query = {
q: arrify(input),
+ format: options.format || isHtml(query.q[0]) ? 'html' : 'text'
}; |
|
Continued in #1647. |
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Fixes #1573
Adds the 'format' parameter to the translate request if specified by caller.