Szpadel / chrome-headless-render-pdf

223 stars 67 forks source link

Add ability to pass extra arguments to Chrome #7

Closed ghost closed 7 years ago

ghost commented 7 years ago

This patch will add the ability to pass desirable options like --no-sandbox and --incognito to the command line options

Szpadel commented 7 years ago

Thank you for this PR!

I would like to wait first for #8, #6 because if we really need to use spawn then using string with all arguments isn't convenient. I'm thinking about passing multiple arguments in this way: --extra-chrome-arg=--no-sandbox --extra-chrome-arg=--force-device-scale-factor=4 This would create array with arguments what could be then just concatenated with existing arguments.

ghost commented 7 years ago

It's your project, so you can do what you like with it. With respect, the way I passed in the extra arguments is standard practice and cleaner.

Szpadel commented 7 years ago

This isn't really just personal preference, but I see here technical challenge.

I had to switch chrome spawn method to spawn and this method expects arguments passed as array. This means that we are then responsible for splitting it on our own. This isn't that easy as splitting it by space character, because you may need to pass space is chrome argument sth, like: --some-arg="some param", split would create in this case 2 arguments --some-arg="some and param" that aren't correct to chrome. Escape sequences like \" wont also work now.

Therefore I see 2 possible options

If you find any better solution I'm open to change my mind :)

ghost commented 7 years ago

I noticed that change. I actually think this will still work as the surrounding string essentially makes --extra-arguments one argument. I'm testing this assumption right now.

ghost commented 7 years ago

Nevermind. It seems to not like it. Not sure why exactly. I'm working on a different solution now...