OmerFlame / SwiftRant

devRant API library made in Swift.
GNU General Public License v3.0
3 stars 1 forks source link

link.calculatedRange wrong for edge case #28

Closed WilhelmOks closed 1 year ago

WilhelmOks commented 1 year ago

I found a comment from ElectroArchiver on devRant (posted 3 hours ago), which has 3 youtube links in it. And for this edge case, the calculatedRange property in Link contains wrong values.

This is a log of the links property in the debugger:

links   [SwiftRant.Rant.Link]?  4 values    some
[0] SwiftRant.Rant.Link 
    type    String  "url"   
    url String  "https://www.youtube.com/watch?v=b05Xtv_ZPFc"   
    shortURL    String? "https://youtube.com/watch/..." some
    title   String  "https://youtube.com/watch/..." 
    start   Int?    28
    end Int?    57
    calculatedRange _NSRange?   some
        location    Int 28
        length  Int 29
[1] SwiftRant.Rant.Link 
    type    String  "url"   
    url String  "https://www.youtube.com/watch?v=lBZeMeZrDEE"   
    shortURL    String? "https://youtube.com/watch/..." some
    title   String  "https://youtube.com/watch/..." 
    start   Int?    59
    end Int?    88
    calculatedRange _NSRange?   some
        location    Int 28
        length  Int 29
[2] SwiftRant.Rant.Link 
    type    String  "url"   
    url String  "https://www.youtube.com/watch?v=_hzaNYYgQb0"   
    shortURL    String? "https://youtube.com/watch/..." some
    title   String  "https://youtube.com/watch/..." 
    start   Int?    90
    end Int?    119
    calculatedRange _NSRange?   some
        location    Int 28
        length  Int 29
[3] SwiftRant.Rant.Link 
    type    String  "mention"   
    url String  "2943442"   
    shortURL    String? nil none
    title   String  "@iiii" 
    start   Int?    nil none
    end Int?    nil none
    calculatedRange _NSRange?   some
        location    Int 0
        length  Int 5

There are 3 url links and one mention link. The calculatedRange has (28, 29) for each of the 3 links, which is wrong. The reason is that SwiftRant calculates the range by searching for the link.title. And the title is the same for all 3 links because it is shortened. So it finds only the first occurence for all of the 3 links.

To fix this, we could just use link.start and link.end to calculate the range. But those are both nil for "mention" type links. So, we need to make a check for that and calculate mention links based on the title.

This is what I did as a workaround in my JoyRant client for now:

            let nsRange: NSRange
            if let start = link.start, let end = link.end {
                nsRange = .init(location: start, length: end - start)
            } else {
                nsRange = (postedContent as NSString).range(of: link.title)
            }

I think this should be implemented in Link.calculatedRange in SwiftRant.

OmerFlame commented 1 year ago

Can you please send a link to the mention? I want to debug this myself in order to figure out the issue, since I've worked on the link features for quite a long time and I didn't know it might be broken on some occasions. It worked on every single mention and link I've seen up to this point...

WilhelmOks commented 1 year ago

I haven't saved the link to that rant, sorry. Maybe you can try to search for that specific urls and find that rant or just create your own comment containing multiple different youtube links.

WilhelmOks commented 1 year ago

I think I found it:

https://devrant.com/rants/6109206/i-m-addicted-to-violent-gory-music-i-don-t-know-why-is-this-bad

OmerFlame commented 1 year ago

Turns out that this is more fundamental than just an edge case.

I wrote the range calculator without adding the scenario where multiple links have the same shortened link in the body into the mix.

I have devised a solution for this issue and I will open a pull request for approving it in a few moments.