oUF-wow / oUF

WoW AddOn - Unit frame framework.
MIT License
222 stars 58 forks source link

tags: look for args in the suffix only #602

Closed Rainrider closed 2 years ago

Rainrider commented 2 years ago

This fixes '[ ($>test:tag<$)]' resulting in (42 instead of (42) in a backwards compatible way.

Here a simple use case for testing in case I missed something:

local function getBracketData(tag)
    -- full tag syntax: '[prefix$>tag-name<$suffix(a,r,g,s)]'
    local prefixEnd, prefixOffset = tag:match('()$>'), 1
    if(not prefixEnd) then
        prefixEnd = 1
    else
        prefixEnd = prefixEnd - 1
        prefixOffset = 3
    end

    local suffixEnd = (tag:match('()%(', prefixOffset + 1) or -1) - 1
    local suffixStart, suffixOffset = tag:match('<$()', prefixEnd), 1
    if(not suffixStart) then
        suffixStart = suffixEnd + 1
    else
        suffixOffset = 3
    end

    return tag:sub(prefixEnd + prefixOffset, suffixStart - suffixOffset),
        prefixEnd,
        suffixStart,
        suffixEnd,
        tag:match('%((.-)%)', suffixOffset + 1)
end

local function getAffixes(tag, prefixEnd, suffixStart, suffixEnd)
    local prefix = tag:sub(2, prefixEnd)
    local suffix = tag:sub(suffixStart, suffixEnd)

    return prefix ~= '' and prefix or nil,
        suffix ~= '' and suffix or nil
end

local tags = {
    '[test:tag]',
    '[prefix$>test:tag]',
    '[test:tag<$suffix]',
    '[prefix$>test:tag<$suffix]',
    '[test:tag(args)]',
    '[prefix$>test:tag(args)]',
    '[test:tag<$suffix(args)]',
    '[prefix$>test:tag<$suffix(args)]',
    '[ ($>test:tag<$)]',
    '[ ($>test:tag<$)(args)]',
}

for _, tag in next, tags do
    local name, pe, ss, se, args = getBracketData(tag)
    local p, s = getAffixes(tag, pe, ss, se)

    print(tag, name, p, s, args)
end

This breaks with something like '[ ($>test:tag<$(suffix)(args)]' which is IMO a limitation of the current tag syntax. We should have placed the argument list before the suffix delimiter. The following solution would fix this but in a backwards incompatible way:

local function getBracketData(tag)
    -- syntax: '[prefix$>tag-name(a,r,g,s)<$suffix]'
    local prefixEnd = tag:match('()$>') or 1
    local prefixOffset = prefixEnd == 1 and 1 or 2
    local suffixStart = tag:match('<$()') or -1
    local suffixOffset = suffixStart == -1 and -1 or -3
    local tagName = tag:sub(prefixEnd + prefixOffset, suffixStart + suffixOffset)
    local args = tagName:match('%((.-)%)')
    if (args) then
        tagName = tagName:gsub(args, ''):sub(1, -3)
    end

    if (prefixEnd ~= 1) then
        prefixEnd = prefixEnd - 1
    end

    return tagName, prefixEnd, suffixStart, -2, args
end
ls- commented 2 years ago

@Rainrider could you add the test stuff to the code? Just leave it there commented out, it could be pretty handy in the future. Or alternatively, add a comment with a link to this PR or Gist with the test code. Also, did you test the strip func? As for changing the tag syntax, I'm open to it, just make a new PR and label it as next major.

Rainrider commented 2 years ago

@ls- Nope, missed the strip ofc. Will take a look later, the weather here is way too fantastic to miss out. As for the test "case" I'd rather squash merge, which will add a ref to the PR, and I'll mention it in the commit message, if you are fine with this.

Rainrider commented 2 years ago

strip should be fine, there is nothing special there and the fix doesn't change the tag syntax, so if it is broken, it was that way before as well.

ls- commented 2 years ago

@Rainrider it'd be nice to have a reminder in the comments that there's a test case people, us included, can take a look at. Like, in 6-12mo I won't even remember what's going on in there 😁 And have fun 🏖

ls- commented 2 years ago

And yeah, strip works just fine, dw

[test:tag]                       ==> [test:tag] 0
[prefix$>test:tag]               ==> [test:tag] 0
[test:tag<$suffix]               ==> [test:tag] 1
[prefix$>test:tag<$suffix]       ==> [test:tag] 1
[test:tag(args)]                 ==> [test:tag] 0
[prefix$>test:tag(args)]         ==> [test:tag] 0
[test:tag<$suffix(args)]         ==> [test:tag] 1
[prefix$>test:tag<$suffix(args)] ==> [test:tag] 1
[ ($>test:tag<$)]                ==> [test:tag] 1
[ ($>test:tag<$)(args)]          ==> [test:tag] 1