Kallithea issues archive

That could make sense. Sounds like an easy fix ;-)

Sounds easy, but still I'm bumping into a stupid problem: I tried changing this line (in templates/pullrequests/pullrequest_show.html)

      linkInlineComments(document.getElementsByClassName('firstlink'), document.getElementsByClassName('inline-comment'));

into

      linkInlineComments(document.getElementsByClassName('firstlink'), document.getElementsByClassName('comment'));

which should work since all inline-comments also have class comment.

However, what I see is that the list of comments passed to function linkInlineComments contains the inline comments twice, and I have no idea why. So as example I have two inline comments (a, b) and three global comments (c,d,e). The list in the second argument to linkInlineComments then contains a1,b1,a2,b2,c,d,e. Where a1 and b1 refer to both classes inline-comment and comment, and a2,b2 only to comment.

I'm by far a Javascript expert, so any help is welcome here. I tried removing the inline-comment class altogether, but for some reason these inline comments are always present twice.

The result of them being present twice is that the first one points to the second, which then points back to the first.

With the above change, the global comments point to each other, and the first one's previous link does indeed point back to the last inline comment, so that part is fine.

(A good example of how fancy markup of technical discussion is annnnnnoying and usually get it wrong unless people take en extra effort to make it "correct").

(This is also something it probably would be better to post on the mailing list so not only reach the few that are stupid enough to browse a bug tracker for fun.)

From the example you show, I can see that this is a part that haven't been jQuery-fiscated yet. Some might dislike it as unnecessary overhead, but I think it makes many things simpler and more predictable. My first step would thus probably be to change this code to jQuery, before trying to improve it.

The reason they appear twice could be that they first appear in one place in the DOM, then some javascript will copy them to the right inline location. (If the right inline location can't be found (because the diff context is smaller than when the comment was created), it should still remain in its old position. That can perhaps be used to find that location and to see if such comments also show twice.)

I guess one solution to the problem could be to specify a css-like selector to jQuery so it only look for comments that actually are inline in the actual diff.

See pull request #81. The problem was indeed related to the moving/copying of inline elements, where the original elements remained. I fixed it by deleting the elements after moving/copying them.

Fixed with commit 990ec5ed4ee5.

The comments in out system doesn't have ANY links to the next comments at all. At the top the bubble brings you to the first comment, and that's it - additional comments we have to manually scroll to view, I could really use that next comment link...

Also I do no think Replies to comments should bump the comment count.