Skip to content

Context-aware user menu (F2) items based on file masks (like in Associations) - #1126

Open
OlegFedko wants to merge 5 commits into
FarGroup:masterfrom
OlegFedko:context-aware-user-menu
Open

OlegFedko wants to merge 5 commits into
FarGroup:masterfrom
OlegFedko:context-aware-user-menu

Conversation

@OlegFedko

Copy link
Copy Markdown

Summary

Implemented Idea #1105: Context-aware user menu (F2) items based on file masks (like in Associations)

Add optional file mask support to the user menu (F2), so menu items can be shown only when the file under the cursor matches the mask (same syntax as file associations: *.json, .json,.xml, etc.).

Main changes:

  • New File masks field in the menu item editor (F4), with validation via filemasks
  • Items without a mask behave as before (always visible)
  • Filtered items are hidden by default when the menu is opened; Ctrl+H toggles showing all items (including filtered-out ones) while editing
  • After Ins/F4, if the edited item would be hidden for the current file, the menu switches to “show all” so the new/edited entry remains visible
  • Masks are stored in FarMenu.ini as an indented @Masks: line (compatible with Alt+F4 text editing)
  • Help updated in all 8 languages (FarEng, FarRus, FarUkr, FarGer, FarPol, FarCze, FarHun, FarSky)

Bug fixes included:

  • Ctrl+Up / Ctrl+Down: selection now follows the moved item instead of staying at the old position

@alabuzhev

Copy link
Copy Markdown
Contributor

Main changes

This is interesting, but can't you use existing facilities for type-specific stuff?
E.g. put json-related commands into json.ini and create an association for *.json like Ctrl+PgDn = lua:mf.usermenu(2, "path_to\json.ini")

Ctrl+Up / Ctrl+Down: selection now follows the moved item instead of staying at the old position

What exactly has been fixed and how to reproduce the issue? I've pressed Ctrl+Up/Down a few times, the moved item stays selected as expected.

@OlegFedko

Copy link
Copy Markdown
Author

This is interesting, but can't you use existing facilities for type-specific stuff? E.g. put json-related commands into json.ini and create an association for *.json like Ctrl+PgDn = lua:mf.usermenu(2, "path_to\json.ini")

You can see my detailed use case in #1105

The reason I want the User Menu specifically, rather than keyboard shortcuts, is that I don't use each individual feature that often.I simply won't remember all of them, meaning I'd need a cheat sheet. Plus, editing these shortcuts is inconvenient.
A menu, on the other hand, is great because you don't have to memorize anything, though it has grown quite bloated over time.
Besides, filters are optional, so if you don't use them, the functionality remains exactly as it was before.Also, since the User Menu has a built-in feature to pass the current file to a command, it's only logical that the menu item becomes redundant when its command doesn't work with that file.
So, this feature is completely logical and will be useful not just for me.
On top of that, using the built-in editor is much more convenient than writing custom Lua scripts, which is something many users don't even know how to do.

Ctrl+Up / Ctrl+Down: selection now follows the moved item instead of staying at the old position

What exactly has been fixed and how to reproduce the issue? I've pressed Ctrl+Up/Down a few times, the moved item stays selected as expected.

My bad! I just checked the release version of Far and realized that I was the one who introduced this bug during development, only to heroically fix it later! 🤦‍♂️

@HamRusTal

Copy link
Copy Markdown
Contributor

I'd like to bring in another dimension here: why limit to the current file? Oftentimes, people have User menu items to handle the entire list of selected panel items. So it would be natural for the mask-based filter to account for these as well. I like to think of this feature as of a FAR-specific context menu. Then, if we have a mixed set of files selected (some match the mask and some not), then the mask-based actions should show but should apply only to the matching files.

And, what if some directories match the mask, too? And what about other attributes? This becomes complicated very soon. Step by step, we risk heading to the full-blown filters support like in File Find…

@yegor-mialyk

Copy link
Copy Markdown
Contributor

I personally do not think the Menu is the right place to implement this. The Apply command (Ctrl+G) seems like a more appropriate place for this feature - something like a context-aware Apply command on steroids, with presets support.

@OlegFedko

Copy link
Copy Markdown
Author

Apply command does not solve this for me. It is useful for running a command over a group of selected files, but it requires typing or remembering a command line. What I want is to choose a named command from a list for the current file. If Apply command were extended with saved named command lists, it would essentially become another User Menu, while the existing User Menu already has the necessary infrastructure for working with selected files.

Treating the F2 menu as a context menu is a concept that will be immediately intuitive to a broad audience, since this exact paradigm already exists in the native file explorers of all major operating systems. It is second nature for most users to expect a "Rotate clockwise" command when interacting with a .jpg file, and a "Play" command for an .mp3 file. Bringing this same context-driven experience to Far would feel completely natural.

I use the User Menu frequently, but I edit it rarely. Therefore, I prefer a slightly more complex one-time configuration over seeing irrelevant menu items every day. For example, when the cursor is on a .csv file, I do not want to see Split PDF: report.csv in the list.

The most natural solution is an optional file-mask filter for individual F2 items, using the same syntax as File Associations, for example .json,.xml. Items without a mask would remain always visible, so the feature would be fully optional and backward compatible.

If masks are considered too limited, a more flexible variant could use power of Special Symbols plus a regular-expression test. For example, Far could build a string from the current file, such as its extension, and show the item only if it matches a regex. This feature could even be hidden inside the Command: field only for advanced users, conceptually like:

Label:
  Fix !.! for Excel

Command:
  show-when-match("!`", "(csv|tsv)")
  fix.exe !\!.!

Where !` is Far's Special Symbol for current file extension.

Ugly but powerful (like already existing !?<title>?<init>! syntax).

@johnd0e

johnd0e commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Partial solution: associations.
When more than 1 available than it shows a choice menu.

@OlegFedko

Copy link
Copy Markdown
Author

Partial solution: associations. When more than 1 available than it shows a choice menu.

Directly using the association via Execute will be too inconvenient.
For example, by default, *.jpg files open in Photo Viewer, and I am fine with that. It would be inconvenient if, all of a sudden, Rotate Clockwise was called instead of Photo Viewer. We could explicitly add both Photo Viewer and Rotate Clockwise so that a menu pops up, but that would make opening files in Photo Viewer cumbersome.

However, if we add a new target User Menu (used for F2) to the associations, and this association appears as an item in the user menu when the cursor is on a matching file, then I would be fine with that.

I am not sure how easy it will be within the User Menu to invoke via F4 another editor for the corresponding association instead of the User Menu editor when hovering over an association.

Also, the User Menu theoretically allows executing multiple commands, whereas you can only insert a single command into an association without jumping through hoops. But I am okay with that.

@HamRusTal

Copy link
Copy Markdown
Contributor

BTW the Rh_Scripts package claims to include a file type-aware User Menu. I did not use it though and cannot comment whether it works.

@MKadaner

MKadaner commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Not exactly Far menu, but text-mode context menu with all the power of the system associations:

image

@OlegFedko

Copy link
Copy Markdown
Author

Not exactly Far menu, but text-mode context menu with all the power of the system associations:

I know about that feature, but I don't like the system context menu because it is overcomplicated. It gets cluttered with tons of menus I never asked for, forced in by third-party installers. To make matters worse, there is no simple way to edit it - you're stuck digging through regedit and manual search.

In contrast, the Far User Menu is a personalized menu tailored to my specific needs, complete with a convenient editor. Furthermore, its ability to use the current file path raises a logical question: why even display an item if it isn't relevant to the current type of file? Having global or folder-specific User Menus was one step towards that - hiding irrelevant items when browsing other folders.

@alabuzhev

Copy link
Copy Markdown
Contributor

Overall the idea of having menu items conditionally visible looks sensible.
It is tempting to have a generic predicate, not just the current file vs masks.
But defining a truly generic one is tricky.

So let's think, what is the user menu typically used for?

  1. Doing something global (either universally applicable or not using the current directory/file).
  2. Doing something specific to the current directory.
  3. Doing something with the current file.
  4. Doing something with the selected files.

The remaining cases are likely too esoteric to mention.

Can these cases benefit from conditionally visible menu items?

1 - irrelevant.
2 - already addressed by local menus.
3 - yes, addressed by this PR.
4 - also yes, but not addressed.

Can we do something about 4?
E.g. use + or some other funny character before the mask to indicate that we're looking at the list of selected files rather than the current file and all of them should match the mask.

@OlegFedko

Copy link
Copy Markdown
Author

The most flexible option is a string using special symbols combined with a regex string applied over it.

A slightly less flexible, but perhaps more intuitive option is to apply regex only over the actual content of the Command: field. After all, certain files must eventually appear in the command, which means you can later use regex to verify that the resulting batch makes sense. There is one exception, however: when the collected paths are placed into a file. But in this case, we could suggest adding rem "files list" as the very first command.

An option that is significantly less flexible, but understandable to most users and likely to cover most real-world scenarios, is a combo box. This would let users select the condition for the menu to appear, combined with a predefined mask:

  • Show always (default)
  • Show if current file matches
  • Show if any selected file matches
  • Show if all selected files match

It is highly unlikely that a menu will have many items that aren't covered by this combo box. In a worst-case scenario, users can tolerate having one extra menu item that can't be perfectly filtered. Furthermore, using a combo box leaves the door open to expanding filtering approaches in the future, if that ever becomes necessary.

@HamRusTal

Copy link
Copy Markdown
Contributor

I like the idea of a combo box paired with a mask specification, rather than using more fancy characters. BUT:

  • Why make checking of the current item and of the selected items mutually exclusive? IMHO these are independent.

  • When we have a menu item whose command(s) involve (meta)symbols, as I suggested earlier,

    if we have a mixed set of files selected (some match the mask and some not), then the mask-based actions should show but should apply only to the matching files.

  • What if the current/selected item is a directory? Masks are not as relevant then, so we likely need to handle this case in a different manner.

@OlegFedko

Copy link
Copy Markdown
Author
  • Why make checking of the current item and of the selected items mutually exclusive? IMHO these are independent.

To keep it simple and intuitive for 99% of cases, rather than maximally flexible but resulting in a huge, complex dialog.

Can you provide a common example of a command where it would be important to check both the current file and the selected ones simultaneously, and where logic that checks the mask for only one of them would yield a clearly poor result?

💡The mere fact that something is selected even without checking the mask (Show when selected: *.*) is already sufficient to ensure it’s almost never visible.

  • When we have a menu item whose command(s) involve (meta)symbols, as I suggested earlier,

    if we have a mixed set of files selected (some match the mask and some not), then the mask-based actions should show but should apply only to the matching files.

I didn’t quite understand the idea. Can you give an example of a command and what kind of filter you’d want to apply to it?

  • What if the current/selected item is a directory? Masks are not as relevant then, so we likely need to handle this case in a different manner.

We could add options to the combo box such as “Show if cursor on directory” and “Show if directories selected”.

For maximum flexibility (and possibly not right now, but just as a point for future expansion), we could also add something like “Show if LUA script returns true”.

@alabuzhev

Copy link
Copy Markdown
Contributor

I like the idea of a combo box paired with a mask specification, rather than using more fancy characters

The combo box state will still have to be serialised into plain text, using more fancy characters or whatever.

@HamRusTal

Copy link
Copy Markdown
Contributor

The combo box state will still have to be serialised into plain text, using more fancy characters or whatever.

Right, but the end user would not need to memorize these, which reduces the unproductive cognitive load.

a command where it would be important to check both the current file and the selected ones simultaneously

A plausible example can be adding multiple files (selected) to an existing archive (current item): the current item determines the archiver program to use, and the various types of files being added determine the compression options.
Another example can be batch conversion of multiple files (selected) according to a preset (current item): if the current item is not a preset or if the type of selected items is incompatible with the preset, the menu item is irrelevant and should be hidden.

I didn’t quite understand the idea. Can you give an example of a command and what kind of filter you’d want to apply to it?

My bad, I have to admit. Here I confused User Menu (where the command is run once) with Apply Command (where the command is run for every selected item, with some exceptions). Let's avoid further scope creep.

We could add options to the combo box such as “Show if cursor on directory” and “Show if directories selected”.

Or we could accompany both the current/selected items' mask with a tri-state check-box “Directories/junctions” ([x] — each matching item must be a directory/junction, [ ] — each matching item must be not a directory/junction, [?] — don't care).

@OlegFedko

Copy link
Copy Markdown
Author

The main goal of the filter is simply to keep the menu at a manageable size without overcomplicating things. If it delivers a reasonable list 99% of the time, that’s good enough for me.

Keep in mind that we can’t completely filter out commands unrelated to files anyway. When a user presses F2, we don't know their exact intent, and there is no distinction between a context menu and a simple shortcut. The filter's job is just to clean up the list, not to guarantee that a command will work.

I don't rule out the existence of highly complex commands with strict requirements for both the current and selected files. Also, the menu allows linking to a passive panel, doubling combinations. However, it seems unlikely that anyone's menu is heavily cluttered with commands that complex.

In most cases, a simple filter will be more than enough to hide them.

adding multiple files (selected) to an existing archive (current item): the current item determines the archiver program to use, and the various types of files being added determine the compression options.

A filter based on the archive itself would work well (since I much more often invoke the menu on things other than archives).
As would a filter based on the fact that something is selected (I rarely open the menu when something is selected).

batch conversion of multiple files (selected) according to a preset (current item): if the current item is not a preset or if the type of selected items is incompatible with the preset, the menu item is irrelevant and should be hidden.

I'd rather filter by preset expansion, since I rarely open the menu when the cursor is on a preset to do something else and it would be hidden in all other cases.

Or we could accompany both the current/selected items' mask with a tri-state check-box “Directories/junctions” ([x] — each matching item must be a directory/junction, [ ] — each matching item must be not a directory/junction, [?] — don't care).

I agree that filtering by files/directories sounds reasonable and could be a popular request. In fact, I can think of a few scenarios where it would be useful.However, I don't think a tri-state checkbox is the best solution for UX. It's not immediately obvious what the unchecked vs. indeterminate states mean, so we would need to add hints. A combo box would probably be a better and clearer approach here.

The combo box state will still have to be serialised into plain text, using more fancy characters or whatever.

Regarding menu serialization - when I was working on the PR, I was quite surprised by the unconventional format (since the INI file doesn't actually follow the INI specification). I even considered migrating it to JSON, but couldn't find any built-in support for it in the codebase at the time, so I just stuck with the existing approach.

It’s hard for me to give a definitive opinion here since I’m not deeply involved in Far Manager development, but JSON has pretty much become the de facto standard for all kinds of configurations nowadays.

[
  {
    "type": "command",
    "label": "Show Hidden Files",
    "hotkey": "H",
    "script": "@echo off\nattrib -h -s *.* /S /D"
  },
  {
    "type": "menu",
    "label": "Developer Tools",
    "hotkey": "D",
    "items": [
      {
        "type": "command",
        "label": "Build Project",
        "hotkey": "B",
        "script": "cd src\nnpm run build"
      },
      {
        "type": "command",
        "label": "Run Tests",
        "script": "@echo off\nnpm test"
      }
    ]
  }
]

Or the new rockstar TOML: INI-light, JSON-strong.

# ==============================================================================
# 💡 TOML SYNTAX NOTE:
# 1. Comments are fully supported using the '#' symbol.
# 2. Indentation is completely optional! It is ignored by the parser.
#    You can indent nested elements for visual clarity, or keep them flat.
# ==============================================================================
[[menu]]
type = "command"
label = "Show Hidden Files"
hotkey = "H"
script = """
@echo off
attrib -h -s *.* /S /D
"""

[[menu]]
type = "menu"
label = "Developer Tools"
hotkey = "D"

  [[menu.items]]
  type = "command"
  label = "Build Project"
  hotkey = "B"
  script = """
  cd src
  npm run build
  """

  [[menu.items]]
  type = "command"
  label = "Run Tests"
  script = """
  @echo off
  npm test
  """

@OlegFedko

Copy link
Copy Markdown
Author

I have made changes based on our discussions.

Now you can specify the menu scope to control when it is displayed:

  • Always (default)
  • Has selection (when items are selected in the panel)
  • Current is a file (optional mask)
  • Current is a folder (optional mask)

The combo box leaves room to easily expand the filters in the future if needed.

Why selection overrides other filters

I decided that if something is selected, the file mask and directory/file checks are not required. Since users can select any mix of items, creating a clear and intuitive filter for that scenario is tricky. Besides, the mere fact that a selection exists is already a powerful filter.

Adding configuration options like When All or When Any match would likely only cause confusion:

  • With When All: If a user accidentally selects an unrelated file, the menu disappears, leaving them confused ("Where did my menu go?").
  • With When Any: It also feels counterintuitive. For instance, if a target .json file is selected along with a bunch of other irrelevant files, the menu still shows up, which feels wrong.
  • Command behavior: Furthermore, if we use When Any, it raises another question: will those target files be filtered out when passed to the command, or not?

Conclusion: I think that for a start, the simple fact that something is selected is a powerful enough filter on its own.

image

Items can be shown always, when there is a selection, or depending on
whether the current item is a file or a folder (optionally with a file
mask). Use Ctrl+H in the menu editor to show or hide filtered items.
Keep selection on the item when moving it with Ctrl+Up/Ctrl+Down.
@OlegFedko
OlegFedko force-pushed the context-aware-user-menu branch from dfcd8b3 to 7226baa Compare September 28, 2026 15:16
…r-menu

# Conflicts:
#	far/changelog
#	far/usermenu.cpp
@OlegFedko

Copy link
Copy Markdown
Author

So everyone, how do you like it? Is there room for further improvement?

Can't wait to roll out Far Manager with this feature across my machines. 🚀💻

@HamRusTal

Copy link
Copy Markdown
Contributor

Is there room for further improvement?

From the above screenshot, the drop-down list items are lacking hotkeys (ideally, assigned in the LNG file rather than automatically).
Also, a snippet of how the new conditions are stored in the FarMenu.ini file might provoke further improvement suggestions.

Disclaimer: I did not try to use the new feature; and I do not have much influence over whether/when the PR gets accepted.

@OlegFedko

Copy link
Copy Markdown
Author

Also, a snippet of how the new conditions are stored in the FarMenu.ini file might provoke further improvement suggestions.

I added a @ShowIf: [combo-mode] [mask-optional-list] macro, like this:

:    PDF Read !.!
     @ShowIf: CurrentFile *.pdf
     f readpdf "!\!.!"

@HamRusTal

Copy link
Copy Markdown
Contributor

I added a @ShowIf: [combo-mode] [mask-optional-list] macro, like this:

Hmm, this syntax conflicts with running a plugin by its command-line prefix with suppressed command echo. E.g. I (really) have this historical user menu item in my menu:

_:   Test
     @lua:print'ver';Keys'Enter'
     python -c "import sys; print(sys.argv)" "!#!\\" EOM

@OlegFedko

OlegFedko commented Oct 7, 2026 •

Copy link
Copy Markdown
Author

Hmm, this syntax conflicts with running a plugin by its command-line prefix with suppressed command echo. E.g. I (really) have this historical user menu item in my menu:

The @ShowIf: prefix is stripped from the command.

So, the only way this could conflict is if you want to suppress an actual ShowIf: command, but I'm not aware of any such real-world commands.

Maybe it would be better if this special line started with a comment marker, like rem ShowIf:, but I honestly don't know the exact syntax for Far command lines. Is rem or :: the comments?

A better solution would be to migrate FarMenu.ini to a more flexible format like json/yaml/toml. I'm ready to do this, but it seems like a rather large change, and I'm not sure I'd get approval for it.

@alabuzhev

Copy link
Copy Markdown
Contributor

Is rem or :: the comments?

Both:

if (!((starts_with_icase(strCommand, L"REM"sv) && (strCommand.size() == 3 || std::iswblank(strCommand[3]))) || starts_with_icase(strCommand, L"::"sv)))

Comments are fine I guess. And backwards compatible.

json/yaml/toml

Please don't.

@OlegFedko

Copy link
Copy Markdown
Author

json/yaml/toml

Please don't.

Haven't changed anything.

Just saw tinyxml.cpp, so adding XML support definitely looks like the easiest option.

Is rem or :: the comments?

Both:

Changed to ::ShowIf:

Will be happy to see my PR in release! 😄

Comment thread far/changelog Outdated
* When passive panel is in non-file mode (Info, etc), SameFolder had no effect
* When passive panel is hidden (Ctrl+F1/F2), SameFolder did not un-hide it

--------------------------------------------------------------------------------

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in the beginning of the file.

Comment thread far/changelog Outdated
* When passive panel is hidden (Ctrl+F1/F2), SameFolder did not un-hide it

--------------------------------------------------------------------------------
olegfedko 2026-09-25 20:00:00+03:00

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update vbuild.m4 accordingly and add the number here.

Comment thread far/changelog Outdated
--------------------------------------------------------------------------------
olegfedko 2026-09-25 20:00:00+03:00

1. User menu items can be hidden by a "Show if" condition: always, has selection, current is a file or current is a folder (with an optional file mask for the last two). Use Ctrl+H in the menu editor to show or hide filtered items.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Ctrl+H in the menu editor

Just "in the menu"

Comment thread far/changelog Outdated

1. User menu items can be hidden by a "Show if" condition: always, has selection, current is a file or current is a folder (with an optional file mask for the last two). Use Ctrl+H in the menu editor to show or hide filtered items.

2. User menu: keep selection on the item when moving it with Ctrl+Up/Ctrl+Down.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, this should not be here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entry moved to the top as build 6745, item 2 dropped, vbuild.m4 bumped.

Comment thread far/FarEng.hlf.m4 Outdated
hot keys in user menu. If #F1# or #F4# is used, its original function in user
menu is overridden. However, you still can use #Shift+F4# to edit the menu.

The #Show if# condition allows you to hide a menu item that is inapplicable in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The #Show if# condition allows you to hide

This "show allows to hide" sounds weird, pls try to reword.

E.g. "allows to show only when applicable" or something.

"Show if: Always" in the UI also sounds weird.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field is now "Show:" (the "if" read oddly with "Always"); help reworded in all languages.

Comment thread far/usermenu.cpp Outdated
UserMenu->SetHelp(L"UserMenu"sv);
UserMenu->SetPosition({ -1, -1, 0, 0 });
UserMenu->SetBottomTitle(KeysToLocalizedText(KEY_INS, KEY_DEL, KEY_F4, KEY_ALTF4, KEY_CTRLUP, KEY_CTRLDOWN));
UserMenu->SetBottomTitle(KeysToLocalizedText(KEY_INS, KEY_DEL, KEY_CTRLH, KEY_F4, KEY_ALTF4, KEY_CTRLUP, KEY_CTRLDOWN));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hint string might get too long already:

╔═════════════════ Local menu ═════════════════╗
║      1                                       ║
╚═ Ins Del Ctrl+H F4 Alt+F4 Ctrl+Up Ctrl+Down ═╝

and Ctrl+H won't do anything unless the user explicitly opted in, maybe don't add it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with this hint? It's not too long, even for a 80-char wide window.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't show Ctrl+H when every menu item is visible (default state for users who do not use this new feature).

Comment thread far/usermenu.cpp Outdated
const auto EnableMask = ShowIfUsesMask(ShowIf);
Dlg->SendMessage(DM_ENABLE, EM_MASK_TEXT, ToPtr(EnableMask));
Dlg->SendMessage(DM_ENABLE, EM_MASK_EDIT, ToPtr(EnableMask));
m_ItemChanged = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe set it only if the combobox selection actually changed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the combobox from DN_EDITCHANGE (it fires even when the same value is set) and kept it in DN_LISTCHANGE, which is only sent on a real change.

Comment thread far/usermenu.cpp Outdated

Label = get_dialog_item_text(Dlg, EM_LABEL_EDIT),
Mask = get_dialog_item_text(Dlg, EM_MASK_EDIT);
const auto ShowIf = MenuShowIfOrder[std::clamp(static_cast<int>(Dlg->SendMessage(DM_LISTGETCURPOS, EM_SHOWIF_COMBO, nullptr)), 0, static_cast<int>(std::size(MenuShowIfOrder)) - 1)];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why clamping here and in similar places?
We can get an invalid index here either due to a coding error or if someone messed up the combobox state in runtime. The latter is not supported and for the former I'd prefer to just crash. Assert maybe?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter is not supported and for the former I'd prefer to just crash.

Even if some script/plugin has messed the combobox state, it would be too cruel to the user to crash and lose e.g. unsaved editors' changes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced with an assert + graceful fallback - assert for programming errors, but no crash in release.

Comment thread far/usermenu.cpp
{ DI_TEXT, {{5, 6 }, {0, 6 }}, DIF_NONE, msg(lng::MEditMenuShowIf), },
{ DI_COMBOBOX, {{5, 7 }, {36, 7 }}, DIF_DROPDOWNLIST | DIF_LISTNOAMPERSAND | DIF_LISTWRAPMODE, },
{ DI_TEXT, {{40, 7 }, {0, 7 }}, DIF_NONE, msg(lng::MEditMenuMask), },
{ DI_EDIT, {{50, 7 }, {DLG_X-6, 7 }}, DIF_HISTORY, },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static "50" leaves a gap after the label:

║ Show if:                                                           ║
║ Always__________________________↓  Mask:     _____________________↓║

It's too much for English, but might be too little for some other language.
Can we calculate the label length in runtime and set the offset accordingly?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mask label/edit offset is computed at runtime (HiStrlen).

Comment thread far/usermenu.cpp
(*MenuItem)->strHotKey = EditDlg[EM_HOTKEY_EDIT].strData;
(*MenuItem)->strLabel = EditDlg[EM_LABEL_EDIT].strData;
(*MenuItem)->Submenu = SubMenu;
MenuItem->strHotKey = EditDlg[EM_HOTKEY_EDIT].strData;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing pointer with reference here is a reasonable change, but it adds extra noise.
I'll change it separately to somewhat reduce the size of this PR.

@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
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