Skip to content

gh-1006: Menu.SetItemExtendedData#1024

Merged
alabuzhev merged 1 commit intoFarGroup:masterfrom
MKadaner:mzk/gh-1006/Menu.SetItemExtendedData
Apr 18, 2026
Merged

gh-1006: Menu.SetItemExtendedData#1024
alabuzhev merged 1 commit intoFarGroup:masterfrom
MKadaner:mzk/gh-1006/Menu.SetItemExtendedData

Conversation

@MKadaner
Copy link
Copy Markdown
Contributor

@MKadaner MKadaner commented Oct 8, 2025

Summary

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

Also, updated Edit.SyncEditorAndFindAllMenu.lua to demonstrate usage of this API.

References

Checklist

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, and to @zg0 for the suggested improvements.

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?

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

@yulian5
Copy link
Copy Markdown
Contributor

yulian5 commented Oct 8, 2025

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
Copy link
Copy Markdown
Contributor Author

MKadaner commented Oct 9, 2025

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

@yulian5
Copy link
Copy Markdown
Contributor

yulian5 commented Oct 9, 2025

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

@MKadaner
Copy link
Copy Markdown
Contributor Author

@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 MKadaner force-pushed the mzk/gh-1006/Menu.SetItemExtendedData branch 2 times, most recently from 055a099 to 1cdd1f4 Compare November 2, 2025 15:56
@MKadaner MKadaner force-pushed the mzk/gh-1006/Menu.SetItemExtendedData branch 3 times, most recently from 26a03ff to 13694f6 Compare November 10, 2025 02:32
@MKadaner MKadaner marked this pull request as ready for review November 10, 2025 02:42
@rohitab
Copy link
Copy Markdown
Contributor

rohitab commented Nov 12, 2025

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
Copy link
Copy Markdown
Contributor Author

MKadaner commented Nov 13, 2025

@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 MKadaner force-pushed the mzk/gh-1006/Menu.SetItemExtendedData branch 2 times, most recently from 8fa887d to 6b3ddb2 Compare November 22, 2025 16:24
Comment thread far/macroapi.cpp
Comment thread plugins/luamacro/luafar/lf_luamacro.c
@MKadaner
Copy link
Copy Markdown
Contributor Author

MKadaner commented Nov 29, 2025

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
Copy link
Copy Markdown

@MKadaner
Copy link
Copy Markdown
Contributor Author

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

@MKadaner MKadaner force-pushed the mzk/gh-1006/Menu.SetItemExtendedData branch from 04d2e9b to 353ea6b Compare April 12, 2026 20:38
@MKadaner
Copy link
Copy Markdown
Contributor Author

Minor updates according to offline discussion with @zg0, rebased to master, and adjusted changelogs and versions.

Ready for review.

@MKadaner MKadaner force-pushed the mzk/gh-1006/Menu.SetItemExtendedData branch from 353ea6b to aa0215b Compare April 12, 2026 20:49
@HamRusTal
Copy link
Copy Markdown
Contributor

offline discussion with @zg0

At the annual Worldwide FAR Manager Developers Conference? :)

@MKadaner MKadaner force-pushed the mzk/gh-1006/Menu.SetItemExtendedData branch from aa0215b to ed18794 Compare April 18, 2026 14:45
Comment thread far/editor.cpp Outdated
1. FarGroupgh-1006: Macro API to set menu item extended data: `Menu.SetItemExtendedData`.
2. Updated `Edit.SyncEditorAndFindAllMenu.lua` to demonstrate usage of this API.
@MKadaner MKadaner force-pushed the mzk/gh-1006/Menu.SetItemExtendedData branch from ed18794 to 5180a05 Compare April 18, 2026 15:47
@sonarqubecloud
Copy link
Copy Markdown

@alabuzhev alabuzhev merged commit 6b7d03b into FarGroup:master Apr 18, 2026
47 of 48 checks passed
@alabuzhev
Copy link
Copy Markdown
Contributor

Thank you

@MKadaner
Copy link
Copy Markdown
Contributor Author

Thank you!

@MKadaner MKadaner deleted the mzk/gh-1006/Menu.SetItemExtendedData branch April 18, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants