Skip to content

Hide MNU_HIDDEN_CHAR prefix from menu title if present - #911

Merged
ychin merged 2 commits into
macvim-dev:masterfrom
eirnym:bracket-menu
Jul 7, 2019
Merged

Hide MNU_HIDDEN_CHAR prefix from menu title if present#911
ychin merged 2 commits into
macvim-dev:masterfrom
eirnym:bracket-menu

Conversation

@eirnym

@eirnym eirnym commented Jun 16, 2019

Copy link
Copy Markdown
Contributor

Closes: #853

@ychin

ychin commented Jun 17, 2019

Copy link
Copy Markdown
Member

This doesn't actually fix the issue. We need to hide the menu completely, not just remove the ']' character.

@eirnym

eirnym commented Jun 17, 2019

Copy link
Copy Markdown
Contributor Author

@ychin it opens as desired when it was requested. This is handled by vim core

@eirnym

eirnym commented Jun 17, 2019

Copy link
Copy Markdown
Contributor Author

Implementation for Win32 has the same check

@eirnym

eirnym commented Jun 17, 2019

Copy link
Copy Markdown
Contributor Author

I'll revisit the issue today

@ychin

ychin commented Jun 17, 2019

Copy link
Copy Markdown
Member

The issue that #853 is complaining about is that any menus that start with "]" need to be hidden in the menu bar. I believe MacVim still shows that at the moment. The name clean up part (which this PR is doing) is mostly for neatness in how it shows the name of the menu, but not the core functionality of what it does.

I think we need to just fix the menus with this prefix to not show up in the main menu bar. gVim / MacVim does something similar with "ToolBar." menus, and MacVim also hides "Touchbar" ones as well.

@eirnym

eirnym commented Jun 17, 2019

Copy link
Copy Markdown
Contributor Author

Thank you, I've found these menus and updated my commit. I'm sorry for inconvenience

@eirnym

eirnym commented Jun 20, 2019

Copy link
Copy Markdown
Contributor Author

I read the logs and don't see any test failed.

@eirnym

eirnym commented Jun 20, 2019

Copy link
Copy Markdown
Contributor Author

@ychin Is it one of flaky tests?

Failures: 
	From test_alot.vim:
	Found errors in Test_suspend():
	Caught exception in Test_suspend(): WaitFor() timed out after 5000 msec @ function RunTheTest[40]..Test_suspend[37]..Stop_shell_in_terminal[3]..WaitFor, line 4

@eirnym

eirnym commented Jun 20, 2019

Copy link
Copy Markdown
Contributor Author

merged current master into my branch

@ychin

ychin commented Jun 24, 2019

Copy link
Copy Markdown
Member

Yes test_suspend is a flaky test and I need to figure out why it's so.

@ychin

ychin commented Jun 24, 2019

Copy link
Copy Markdown
Member

I don't think the current implementation does what Vim is supposed to do? "]" is not a substitute for "PopUp". It just hides the menu. You can then use :popup to pop up any menu including the ones you hid using ], or just a regular menu like "File". We shouldn't have any code that assumes "PopUp" menus have the same meaning as "]" ones.

@eirnym

eirnym commented Jun 24, 2019

Copy link
Copy Markdown
Contributor Author

there was two reasons for this:

  1. PopUp are just special hidden menus as it's documented, ] are user-defined. I haven't found any other difference between them
  2. ObjC implementation says that they follow the same logic in terms of creating and processing NSMenu and NSMenuItem objects. Toolbar and Touch Bar are processed elsewhere. Do you really want to create a few more methods with the same logic?
    // If descriptor has no parent and its not a popup (or toolbar) menu,
    // then it must belong to main menu.

@ychin

ychin commented Jul 7, 2019

Copy link
Copy Markdown
Member

Yeah sorry you are right. I misread the code.

@ychin
ychin merged commit 2760686 into macvim-dev:master Jul 7, 2019
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.

menu bar not hide ']' prefix item

2 participants