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

Quick reply pluralisation is incorrect when going from 0 to 1 and 1 to 2 comments #1626

Closed
sgsabbage opened this issue Nov 9, 2015 · 9 comments

Comments

@sgsabbage
Copy link
Contributor

When going from 0 to 1 comments, the comments link says '1 comments'

When going from 1 to 2 comments, the comments link says '2 comment'.

@rahaeli
Copy link
Contributor

rahaeli commented Nov 9, 2015

That's either a S2.core problem or a problem with QR not feeding the translation system the right parameters (there's a thing that has to do with pluralizing but God knows if I can remember it). I'm guessing the latter is more likely.

@sgsabbage
Copy link
Contributor Author

Really quick note to anyone who may take this on (including me if my time frees up in the future) as I was glancing over it:

https://github.com/dreamwidth/dw-free/blob/3868ae55348e08737ad2eba430b504d90b649f06/htdocs/js/jquery.quickreply.js#L123

Seems to be the line in question, doesn't look like it's touching the translation system at all.

@kareila
Copy link
Member

kareila commented Nov 17, 2015

The comment link text appears to be an S2 property. The line of code indicated above uses a regular expression to update the digits in that text to indicate the new comment count.

It looks like the simplest way to proceed with this is to add another conditional regular expression, where if the new comment count is 1, change 'comments' to 'comment', otherwise change 'comment' to 'comments' (but only if it didn't say 'comments' before, otherwise you'll end up with 'commentss').

Unfortunately, if someone customizes their style to say something like 'N people have left comments', this still won't do the right thing. So we have to weigh the trade-off of making the grammar correct for the 99.99% of users who use the default text vs. unpredictable behavior for the other 0.01%.

@rahaeli
Copy link
Contributor

rahaeli commented Nov 17, 2015

I feel like there's already a solution to this. Doesn't S2 already do the right thing with plurals? I remember it being a big thing since other languages do plurals differently and we had to come up with a way to make S2 (and the translation system as a whole) understand them...

@kareila
Copy link
Member

kareila commented Nov 17, 2015

Yes, S2 deals with plurals, but when we update the comment count, we don't use S2 to reprint the link - instead jQuery is used to update the count in the rendered string.

@sgsabbage
Copy link
Contributor Author

Bit of a far out suggestion - is it something we could return from the AJAX call, the correct text to display? I haven't at all looked at what the call is made to, or if it would support it, but just thinking out loud.

@kareila
Copy link
Member

kareila commented Nov 18, 2015

That's along the lines of what I would look into, yeah. Don't know how tricky it would be to add, but worth a shot.

@kaberett
Copy link
Contributor

NB if you comment on an entry 0->1 you see "1 comments"

if you comment on that same entry again without refreshing the page to bump the comment count 1->2 you see "2 comments" (correct behaviour!)

@sgsabbage
Copy link
Contributor Author

Claimed

sgsabbage added a commit to sgsabbage/dw-free that referenced this issue Feb 20, 2016
zorkian added a commit that referenced this issue Mar 25, 2016
Fixes #1626 by storing the different plural phrases on the comment link
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants