Hi Sean,

Thanks for your feedback!  (responses below)


On 12 November 2017 at 18:11, Sean P. DeNigris <s...@clipperadams.com> wrote:
> Alistair Grant wrote
>> https://github.com/akgrant43/Pharo-Chrome
>
> Wow, that was a wild ride!

Sorry about that.


> Lessons learned along the way:
> 1. On a Mac, to use the snazzy `chrome` terminal command referenced all over
> the place in the docs, you must first `alias chrome="/Applications/Google\
> Chrome.app/Contents/MacOS/Google\ Chrome"`

I'm an Ubuntu Linux user, however if you look at OSXChromePlatform
class>>defaultExecutableLocation you can see that is where it should
be looking for the exe, so the alias shouldn't really be necessary.
Torsten wrote this, so maybe has more insight.


> 2. Chrome must be started with certain flags: `chrome
> --remote-debugging-port=9222 --disable-gpu` (not sure if the last flag is
> needed, but `#get:` seemed to hang before using; reference
> https://developers.google.com/web/updates/2017/04/headless-chrome)

I've been using this without headless mode.  I'll add a headless flag
that also disables the gpu.



> 3. Beacon has renamed InMemoryLogger to MemoryLogger
> 4. I guess Beacon has renamed `#log` to `#emit`

Sorry about that.  I didn't realise that the Pharo-Chrome baseline is
loading Beacon stable while my install script upgrades it to
#development.  #development is more recent, so I'll update the
baseline.



> 5. I had to comment out `chromeProcess sigterm.` because `chromeProcess` was
> nil and also #sigterm seemed not to be defined anywhere in the image. I'm
> not sure what the issue is there.

chromeProcess is set in GoogleChrome>>openURL:.  Can you give me a
small example that demonstrates the problem?

#sigterm is implemented by OSSUnixSubprocess, which is what I
ultimately use to launch the Chrome process on Ubuntu.

But... this will be broken on Mac at the moment because the current
method of launching chrome doesn't keep track of the process, so
doesn't support #sigterm.  Do you know if OSSUnixSubprocess works on
Mac?  If it does, I can update the code (but not test it :-().


> Pull request issued for #3 & #4.

Once I update the baseline this shouldn't be required.


> Also, I'm not sure what platforms you
> support, but you may want to tag the example methods with <gtExample> or
> similar so that they are runnable from the browser and open an inspector if
> there is an interesting return value.

Good idea, I'll do this.

I'm also making a few other changes:

1. Add an #extractTables method that searches through the page and
returns an array of rows for each table it finds in the page
(something that can easily be loaded in to DataFrame using #fromRows:,
but I don't want to make Pharo-Chrome dependent on DataFrame at the
moment).  Most of the time I use Pharo-Chrome it is extracting data
from tables.

2. I don't know of any reliable way to tell when a page has loaded
since there can always be javascript that periodically updates the
page.  At the moment it waits until the page hasn't changed for a
configurable amount of time.  I'm planning to add a check for specific
content to determine if the page is considered loaded.

3. Add some documentation to the readme :-)



> -----
> Cheers,
> Sean

I'll let you know when I have a new version available (hopefully in
the next few days).


Thanks again,
Alistair

Reply via email to