S2 Max Recursion and Comment Threads

[wyntarvox]

I understand why the max recursion limit exists, but at the moment it's sort of ugly in its execution with regard to comment threads.

For example, if a comment thread exceeds the limit the layer dies as expected. However, there's no obvious indication beforehand that it's going to happen, so someone doing something as innocent as commenting effectively ruins the remainder of the comment threads on the entry. Now, you can see the rest of the comments if you view the entry in a site scheme, but that's not immediately obvious to Joe User.

I'll be the first to admit that I'm definitely no programming expert so I may be asking the impossible... Can we only kill the function that causes the recursion limit in a way that'll let the remaining comment threads on an entry get printed? It was also mentioned to me that perhaps we could look in to printing the comment threads iteratively rather than recursively? At the least, can/should we prevent people from making the nth comment that'll hit the recursion limit and therefore break the rest of that entry's comments?

Comments

[pauamma]

IIRC, there's a sitewide limit on thread depth, independent of styles. Like, 150 or so? Maybe we could adjust the recursion limit to rely on that? (Assuming the problem is because it currently uses a lower value.)

[afuna]

I think we might be able to at the very very least make it print out a "this thread is too long to be handled by S2. Please try viewing it in the light scheme ___?format=light" along with the error message

[mark]

I think I'd be okay raising it a lot.

Something to check: see if the timeout on S2 running works if you allow S2 to go into an infinite loop. If it does, we may be able to get rid of that limitation entirely, or at least, make the limit obscenely high (150,000 instead of 150, f.ex.) then nobody will ever see that error on comments.

[animone]

Somewhat related.. I'll go ahead and ask a lot here and outline what I think could be an ideal situation. I'd love to see collapsed threads (yes, I do understand the need for them, especially when they get very long/in-depth) be user-expandable by some ajax-y method. Unnfortunately, I don't see any good way of implementing this across all styles. I'd also like to see the limit at which threads auto-collapse be a user-defineable variable, of course within reason. The default could be left relatively small, with increased ease of expanding them.

[jimmyb]

It's funny, I asked this in lj_dev this morning I think...it would be nice to see it die nicely instead of that relatively cryptic message which "Joe User" is obviously not going to understand.