kmarius / jsregexp

JavaScript regular expressions for Lua
MIT License
28 stars 4 forks source link

Remove `__call` metamethod for compiled regexp objects #23

Open kmarius opened 2 months ago

kmarius commented 2 months ago

Pinging @L3MON4D3

I would like to remove the old, undocumented __call metamethod at some point in the future because it duplicates a lot of the matching logic.

As far as I can tell, LuaSnip only uses the begin_ind property and the capture groups of a match. A simple migration step could look like this: when loading jsregexp, check if the __call metamethod exists, and if not set to something like this:

local jsregexp = require("jsregexp")
local meta = getmetatable(jsregexp.compile(""))

meta.__call = function(self, str)
    local match = self:exec(str)
    if not match then
        return {}
    end
    return {
        {
            begin_ind = match.index,
            groups = { unpack(match) },
        },
    }
end

This should work as long as only the first match is needed and the global flag is not set. A full replacement looks more like this (Note that the match object already contains the full match in match[0] which could be used directly by LuaSnip):

meta.__call = function(self, s)
    local jstr = jsregexp.to_jsstring(s)
    self.last_index = 1
    local matches = {}
    while true do
        local match = self:exec(jstr)
        if match == nil then
            break
        end
        table.insert(matches, {
            begin_ind = match.index,
            end_ind = match.index + #match[0] - 1,
            groups = { unpack(match) },
            named_groups = match.groups,
        })
                if not self.global then
                        break
                end
        if #match[0] == 0 then
            self.last_index = self.last_index + 1
        end
    end
    return matches
end

Opinions? I am not planning to release anything until I have the OK, and there is no urgency whatsoever.

L3MON4D3 commented 1 month ago

Hi, thanks for pinging me :) This sounds very reasonable, and we already have a small facade around jsregexp, so the __call-addition will slot right in :D Right now we suport both jsregexp 0.0.6 or 0.0.5, are there bugfixes/features relevant to luasnip in newer versions (ie. would we even have to upgrade away beyond 0.0.6)?

kmarius commented 1 month ago

Hi, thanks for pinging me :) This sounds very reasonable, and we already have a small facade around jsregexp, so the __call-addition will slot right in :D Right now we suport both jsregexp 0.0.6 or 0.0.5, are there bugfixes/features relevant to luasnip in newer versions (ie. would we even have to upgrade away beyond 0.0.6)?

There are occasional fixes for libregexp and updates to unicode: Changelog The problem with requiring old versions will always (?) be nixpkgs, i.e. https://github.com/L3MON4D3/LuaSnip/issues/1139

L3MON4D3 commented 1 month ago

Ah, good points :+1: Updating once every few months isn't an issue, so definitely feel free to drop the __call-metamethod, I'll add your fix when we update jsregexp, or sooner if there's some reason :D