edsu / anon

tweet about anonymous Wikipedia edits from particular IP address ranges
Creative Commons Zero v1.0 Universal
973 stars 152 forks source link

Would this bot be more useful if it tweeted screenshots of the diff? #151

Closed Glench closed 6 years ago

Glench commented 6 years ago

As it stands, all tweets shown in the feed have no distinction between things like simple typo fixes:

screenshot 2017-10-12 11 33 30

Or edits to factual information:

screenshot 2017-10-12 11 35 32

I was wondering if it would be a useful feature to have anon capture a screenshot of the wikipedia diff to post alongside the tweet. That way, a person following an anon bot can see a preview of the edit in their feed without having to click through.

edsu commented 6 years ago

Excellent idea! It ought to be doable. If you want to give it a whirl I'd be happy to look at a pull request. If not I think I can get to it sometime.

Glench commented 6 years ago

I took a crack at it here but don't know how to test that it's working.

edsu commented 6 years ago

One thing you can do is ask Google what your ip address is and configure anon to listen for anonymous edits from your IP range and post to your Twitter account (or another one). Then go and anonymously add a space to a Wikipedia page :-)

edsu commented 6 years ago

It would be useful to get the code working with Mastodon as well. But I can add that in if webshot is working well for you.

Glench commented 6 years ago

Okay, I added some code for Mastodon here, but haven't tested either Twitter or Mastodon yet. I'll test them out soon.

Out of curiosity, are there any wikipedia edits that don't have the diff box at the top of the page? This is one of the only ways I can see the new code failing (besides trivial errors).

Glench commented 6 years ago

For some reason webshot is clipping the screenshot. I opened an issue but it looks like that library hasn't been maintained. It's possible to just use phantomjs, but I'd have to look at it later. It's pretty easy if you'd like to do it instead, @edsu.

edsu commented 6 years ago

Thanks very much for testing! I have done screenshots with phantomjs for another project so it's potentially something I can do when I can find the time.

Glench commented 6 years ago

Okay, I ended up using just phantomjs and the same thing happened; the screenshot was truncated in the wrong place. However, with a fudge factor that I had to derive empirically, screenshots now look good and are working for Twitter! I don't have a Mastodon account, but maybe you can test that? I think after that this code is ready to be merged.

Glench commented 6 years ago

You can review the full changes here.

edsu commented 6 years ago

That's awesome! Do you have any example tweets I can see?

If you send a PR I can make some comments in there. It looked like you were taking a screenshot twice when posting to Twitter and Mastodon? Also I wasn't entirely clear why the tweet and inspect functions were made async.

Glench commented 6 years ago

Here's an example tweet: https://twitter.com/Glench/status/920681462319415297

Oh, I didn't think about the case where one posts to both Mastodon and Twitter! But I think having two calls simplifies things. The main case that it simplifies is removing the screenshot. Since there's all this asynchronous stuff happening, it becomes hard to coordinate both async functions being done before the screenshot can be deleted. The cost of taking two screenshots is that it takes a little longer, makes an extra network request, and adds another image to disk temporarily — all tradeoffs worth making to make the code much simpler I think.

Those functions had to be made async because 'await' only seems to work in functions that are marked as async. Here's what happens without marking tweet and inspect async:

screenshot 2017-10-18 12 06 03

Also, I had to update Twit because I was seeing errors that new versions of the library fix.

edsu commented 6 years ago

But you don't directly await tweet() or inspect() right? I guess you have to add async to those because they contain the call to takeScreenShotOfDiff()?

edsu commented 6 years ago

Thanks for the PR!

Glench commented 6 years ago

But you don't directly await tweet() or inspect() right? I guess you have to add async to those because they contain the call to takeScreenShotOfDiff()?

Exactly.

edsu commented 6 years ago

Fixed as part of #155