headzoo / surf

Stateful programmatic web browsing in Go.
MIT License
1.48k stars 159 forks source link

Feature/history trimming #69

Closed jtwatson closed 7 years ago

jtwatson commented 7 years ago

Add ability to cleanup held resources by ether calling Close() on the Browser when it is no longer needed, or optionally having the ability to clear the browser history with ClearHistory()

Implemented ability to set a maximum history length that will be enforced as the history grows.

lxt2 commented 7 years ago

I think this has been rebased incorrectly @jtwatson. Try:

git reset --hard origin/master
git cherry-pick e8466cc
git push --force
lxt2 commented 7 years ago

Urgh, nope, scratch that, e8466cc isn't correct.

lxt2 commented 7 years ago

The other approach is:

git fetch
git reset --mixed origin/master

That will leave you with your changes uncommitted on top of origin/master and then you can commit them as makes sense and push up the changes.

lxt2 commented 7 years ago

Apologies if git pointers aren't useful :)

jtwatson commented 7 years ago

I was rebased to dev and acidentally created PR from master. Do we want to work off of dev or master?

headzoo commented 7 years ago

@jtwatson - We're supposed to work off the dev branch, but we haven't been very strict about enforcing the rule.

jtwatson commented 7 years ago

Okay, I fixed the base branch so we should be all set.

lxt2 commented 7 years ago

Looks good to me, seems sensible to have some defaults.

jtwatson commented 7 years ago

I am not so sure how others are using surf, but for me it seems that I would never go back into history 100 items. If we set the default to 100, what is the likelihood that we would cause a BC problem?

I almost think that it is more likely that someone will encounter problems from high memory usage... It is an easy fix once you realize what is happening, but a default of 100 would avoid the issue completely.

jtwatson commented 7 years ago

@headzoo what is your opinion on using a history default that is something less than unlimited?

headzoo commented 7 years ago

Well, the number of devs trying to figure out why the history suddenly works unexpected is probably much lower than those wonder why Surf is eating up memory like candy. So, I'm fine with using 100 as the default limit. But we should do a patch release for the change.