gh-1006: Menu.SetItemExtendedData

[MKadaner]

Summary

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

References

• Issues with new Find all results menu #1006

Checklist

• I have followed the contributing guidelines.
• I have discussed this with project maintainers: Issues with new Find all results menu #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?

[yulian5]

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.

[MKadaner]

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

[yulian5]

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

[MKadaner]

@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.

[rohitab]

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.

[MKadaner]

@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 })

[alabuzhev]

What is -3?
Maybe introduce some named constant or add a comment?

[shmuz]

What is -3?
Maybe introduce some named constant or add a comment?

It is table position in Lua stack.
Stack position is often expressed as a negative value relative to stack top.
When we want to set a key-value pair in a table we first push the key then push the value.
If initially the table was on Lua stack top its stack position was -1.
After key and value were pushed the table position became -3.

[MKadaner]

Thank you for the explanation, @shmuz.

@alabuzhev, as you can see, it is not a magic constant. I'll add a comment.

[alabuzhev]

pos >= 1 && pos <= top - 1 -> pos > 0 && pos < top?

[MKadaner]

Elements in the Lua stack are numbered starting from 1:

The first element in the stack (that is, the element that was pushed first) has index 1, the next one has index 2, and so on. We can also access elements using the top of the stack as our reference, using negative indices. In that case, -1 refers to the element at the top (that is, the last element pushed), -2 to the previous element, and so on.

So, pos >= 1 is a sanity check. Given how pos is calculated above (int pos = param >= 0 ? param : top + 1 + param;), we could say pos != 0. However, I followed the style established by FMVT_SETTABLE and FMVT_GETTABLE handlers above. Also, I believe that we should not reference cbdata->start_stack (unlike in FMVT_STACKPOP handler), because the pos here is the position of the table, which can probably be below the part of the stack provided by Lua to our C code.

As for the second part (pos <= top - 1), since lua_next expects one value at the top of the stack (the Key), the table cannot be at the top.

I can add all these explanations to the comment in the code, but I am not sure it would be a good idea. Let's keep it as is.

[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.

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
4.3% Duplication on New Code

See analysis details on SonarQube Cloud

[MKadaner]

@rohitab, may I ask you to kindly remind the motivation for Menu.SetItemExtendedData? I am currently discussing (offline) with one of the maintainers whether this function should be added at all. General Far philosophy is to add features only if they are actually needed / requested. Thus, the motivation for this new function is of substantial interest.

It would also be great if you could share the macro we discussed in #1006 and which inspired this new function, so that the maintainer could get better idea of what functionality cannot be properly supported without Menu.SetItemExtendedData. No need to cleanup anything, and you can share privately if you have any concerns.