moshen / node-googlemaps

A simple way to query the Google Maps API from Node.js
MIT License
559 stars 148 forks source link

binary request for static maps #119

Open cmyip opened 8 years ago

cmyip commented 8 years ago

This fixes staticMaps function by properly setting request configuration

fabriziomoscon commented 8 years ago

Hi @cmyip thanks for contributing. Could you please point me to the documentation page so I can check the binary parameter and possibly add some info to the README?

cmyip commented 8 years ago

hi @fabriziomoscon no reference found, but it was a stack overflow question: http://stackoverflow.com/questions/14855015/getting-binary-content-in-node-js-using-request

fabriziomoscon commented 8 years ago

I found a test https://github.com/request/request/blob/master/tests/test-body.js#L51-L55 and docs

encoding - Encoding to be used on setEncoding of response data. If null, the body is returned as a Buffer. Anything else (including the default value of undefined) will be passed as the encoding parameter to toString() (meaning this is effectively utf8 by default). (Note: if you expect binary data, you should set encoding: null.)

I think this is the relevant source code from request: https://github.com/request/request/blob/master/request.js#L1012-L1045

Although I think it is counter intuitive to set something to binary to achieve a null value. Perhaps we could consider a condition that checks whether encoding is null (since it is a function parameter it is null only if the user has specified it so, opposite to undefined where the user omits it).

if (encoding == null) {
    options.encoding = null;
} else if (encoding) { // we check this so we don't assume that request is checking for undefined
    options.encoding = encoding;
}

What do you think?