FarManager icon indicating copy to clipboard operation
FarManager copied to clipboard

gh-1006: `Menu.SetItemExtendedData`

Open MKadaner opened this issue 2 months ago • 8 comments

Summary

Added Lua API B=Menu.SetItemExtendedData([hDlg,][N,]T).

References

  • gh-1006

Checklist

  • [ ] I have followed the contributing guidelines.
  • [X] I have discussed this with project maintainers: gh-1006
    If not checked, I accept that this work might be rejected in favor of a different great big ineffable plan.

Details

The implementation is ready for review, complete with the support in the Editor. Special thanks to @shmuz for the great help with passing Lua tables as parameters to native functions.

When all comments are addressed, I will update all changelogs and version files.

Now, one can write in Lua:

Menu.SetItemExtendedData(FarDialogEvent.hDlg, SelectPos, { Line = 496, Position = 28, Length = 6 })

@rohitab, could you please test the new function with your macro?

@shmuz, could you please look at the changes related to the new function and passing table as a parameter?

@alabuzhev, could you please look at Editor and VMenu?

MKadaner avatar Oct 08 '25 01:10 MKadaner

Hi @MKadaner,

Did you try Copilot? Here is a result (see below 2 solutions to convert table to key/value arguments):

Accepting a table for extended data

Modify Menu.SetItemExtendedData to accept either the existing vararg key/value form or a single table for the data. Detect the argument type and normalize to a table inside the function. Implementation sketch:

-- Old usage (existing):
-- Menu.SetItemExtendedData(hDlg, itemPos, "Line", 496, "Position", 28, "Length", 6)

-- New usage you want to support:
-- Menu.SetItemExtendedData(hDlg, itemPos, { Line = 496, Position = 28, Length = 6 })

function Menu.SetItemExtendedData(hDlg, itemPos, ...)
    local info = {}

    local third = select(1, ...)
    if type(third) == "table" and select("#", ...) == 1 then
        -- Called with a single table: normalize directly
        info = third
    else
        -- Called with vararg key/value pairs: convert to table
        local n = select("#", ...)
        if n % 2 ~= 0 then
            error("SetItemExtendedData expects key/value pairs or a single table")
        end
        for i = 1, n, 2 do
            local k = select(i, ...)
            local v = select(i+1, ...)
            if type(k) ~= "string" then
                error("SetItemExtendedData keys must be strings")
            end
            info[k] = v
        end
    end

    -- Now use `info` as the data table
    -- Example: store it in dialog item extended data
    -- (Replace the following with existing storage logic)
    -- internal_store_extended_data(hDlg, itemPos, info)
end

Backward compatibility and notes The function above preserves the old calling convention and adds the new table-style call. It validates keys are strings and enforces even vararg length. Replace the placeholder storage call with your current code that actually attaches info to the dialog item. Example usage after change: Old: Menu.SetItemExtendedData(hDlg, pos, "Line", 496, "Position", 28, "Length", 6) New: Menu.SetItemExtendedData(hDlg, pos, { Line = 496, Position = 28, Length = 6 })

Then:

The built-in Far Manager Lua macro API does not automatically convert a single table into the keyed varargs form — you must pass the data in the form the API expects or convert your table to key/value varargs before calling Menu.SetItemExtendedData.

Far’s API functions that accept key/value pairs from Lua are usually implemented to receive varargs, not a single table. If the C-side (or existing Lua wrapper) does not explicitly detect and accept a table, calling Menu.SetItemExtendedData(hDlg, pos, { Line = 496, Position = 28, Length = 6 }) will not work as-is.

Two simple options

  1. Convert the table to varargs at call site

Use a small helper that flattens the table into alternating key/value arguments and call the API with table.unpack:

local function table_to_kvlist(t)
    local out = {}
    for k, v in pairs(t) do
        out[#out+1] = k
        out[#out+1] = v
    end
    return out
end

local data = { Line = 496, Position = 28, Length = 6 } Menu.SetItemExtendedData(FarDialogEvent.hDlg, SelectPos, table.unpack(table_to_kvlist(data)))

This requires no changes to Far or to existing API wrappers and preserves backward compatibility. 2) Add a Lua wrapper that accepts a table (preferred if you can change macro helpers)

If you can add a small Lua wrapper function in your macros library, normalize both calling forms and forward to the API:

function SetItemExtendedDataSafe(hDlg, pos, ...)
    local info
    if select("#", ...) == 1 and type(select(1, ...)) == "table" then
        info = select(1, ...)
        local kv = {}
        for k, v in pairs(info) do
            kv[#kv+1] = k
            kv[#kv+1] = v
        end
        Menu.SetItemExtendedData(hDlg, pos, table.unpack(kv))
    else
        Menu.SetItemExtendedData(hDlg, pos, ...)
    end
end
-- usage:
SetItemExtendedDataSafe(FarDialogEvent.hDlg, SelectPos, { Line = 496, Position = 28, Length = 6 })

This keeps old code working and gives you the nicer table call everywhere else. If you control the API implementation

If you are writing or can modify the C-side binding or existing Lua wrapper for Menu.SetItemExtendedData, implement the normalization there (detect a single table argument and convert it to key/value pairs). That is the cleanest solution but requires changing the API wrapper code. Recommendation

If you cannot change the API implementation, add the small Lua wrapper (option 2). If you can change the API, add table acceptance at the wrapper layer so both forms are supported natively.

yulian5 avatar Oct 08 '25 22:10 yulian5

Thank you, yulian5. It seems to be a correct solution for a wrong problem.

MKadaner avatar Oct 09 '25 01:10 MKadaner

@MKadaner, I tried to help and used PR description. What did I understand wrong?

yulian5 avatar Oct 09 '25 02:10 yulian5

@yulian5, here I am actually "writing ... the C-side binding" for Menu.SetItemExtendedData. This is the only useful thing our intellectual friend has suggested. This task is not so easy, because, as I explained in the PR description, "currently there is no way to pass a table parameter." Supporting table parameters is not entirely straightforward. I am currently discussing possibilities with the code owners.

Other suggestions of our artificial friend are as obvious as they are awkward. Having Lua wrappers transforming tables to KVPs is not an option, mostly from aesthetics perspective.

MKadaner avatar Oct 10 '25 00:10 MKadaner

Now, one can write in Lua:

Menu.SetItemExtendedData(FarDialogEvent.hDlg, SelectPos, { Line = 496, Position = 28, Length = 6 })

@rohitab, could you please test the new function with your macro?

I couldn't get this to work. Maybe I'm not doing it right. After calling Menu.SetItemExtendedData, I don't see any change in the menu. The information displayed is exactly the same as before, i.e., the one displayed before the call. However, if I call Menu.GetItemExtendedData after setting the data, it returns the data that was set. If I now press F4, the selection is correct as well, and matches the Line, Position and Length that was passed to Menu.SetItemExtendedData.

rohitab avatar Nov 12 '25 21:11 rohitab

@rohitab

I couldn't get this to work...

I apologize. I've spent too much time in this feature, so it's all obvious to me, and I failed to clearly specify the expected behavior. What you observed is exactly what is implemented. The new Lua function sets the extended data, it is observable with Menu.GetItemExtendedData, and if you add a menu item and set some contrived extended data, the corresponding Editor line will be included into the "Filtered" editor opened by F4.

So, I would say everything works. Thank you very much for testing.

To add some details. The appearance of the menu item is not affected by Menu.SetItemExtendedData. Neither text nor fixed columns (line / position) change. I am going to make it work after this change is merged. I will remove the contents of the fixed columns from the item text and will generate fixed columns on the fly. Item text will be exact copy of the source Editor line and will never change.

P.S. BTW, these should work as well:

-- Current menu
Menu.SetItemExtendedData(SelectPos, { Line = 496, Position = 28, Length = 6 })

-- Currently selected item
Menu.SetItemExtendedData(FarDialogEvent.hDlg, { Line = 496, Position = 28, Length = 6 })

-- Currently selected item in the current menu
Menu.SetItemExtendedData({ Line = 496, Position = 28, Length = 6 })

MKadaner avatar Nov 13 '25 00:11 MKadaner

I added a comment explaining enigmatic -3 and addressed one SonarQube warning.

All other SonarQube warnings are either require rather substantial refactoring (like using instead of typedef in the entire plugin.hpp) or need to be considered in the wider context of HTML documentation formatting. In either case they do not belong in this PR.

MKadaner avatar Nov 29 '25 15:11 MKadaner