Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Converting &apos and ' to an apostrophe #72

Closed
wants to merge 1 commit into from

Conversation

nloadholtes
Copy link
Contributor

Sometimes toots come through that don't have the semicolon. This should get
both cases.

Sometimes toots come through that don't have the semicolon. This should get
both cases.
@ihabunek
Copy link
Owner

I haven't encountered this. Does this happen on a specific instance? Sounds a bit strange.

@nloadholtes
Copy link
Contributor Author

Good question. I'll go and see if I can find the account where this was happening. (I didn't post it originally out of respect to privacy of the user whose name caused the issue.)

@ihabunek
Copy link
Owner

ihabunek commented Jan 2, 2019

@nloadholtes Can you check if the change proposed in #83 helps your use case?

@nloadholtes
Copy link
Contributor Author

I don't think it does. Here's a screenshot of a toot I made this morning with an apostrophe in it, as seen via the curses view:
image

https://octodon.social/@nloadholtes/101347405963562119

Now for the crazy part: I see that on both master and on my branch. 🤦‍♂️

I'm starting to think this PR might not be good/correct. If I can't figure this out in 24hrs, I'll close the pr.

@dlax
Copy link
Contributor

dlax commented Jan 2, 2019

@nloadholtes the content of this toot as retrieved through the API at https://octodon.social/api/v1/statuses/101347405963562119 contains ' (with trailing ;). So the fix from #83 should help I think.

@nloadholtes
Copy link
Contributor Author

@dlax I totally agree it should, I'm just puzzled why I saw that when I tested. I'm 99% sure my brain is still in holiday mode so I've missed something. 😉

I'm going to go ahead an close this PR, I think the code from #83 is a better choice, if nothing else it has tests and I don't. 😄

@nloadholtes nloadholtes closed this Jan 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants