New issue
Advanced search Search tips

Issue 792594 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Windows: rc.py does not support MENUITEM SEPARATOR

Project Member Reported by marshall@chromium.org, Dec 6 2017

Issue description

Chrome Version: master revision 5fdc0fab2 (#520840)
OS: Windows 10 64-bit

What steps will reproduce the problem?
(1) Compile an .rc script that contains a MENUITEM SEPARATOR statement like:

IDC_CLIENT MENU
BEGIN
    POPUP "&File"
    BEGIN
        MENUITEM "&Find...",                    ID_FIND
        MENUITEM SEPARATOR
        MENUITEM "E&xit",                       IDM_EXIT
    END
 END

What is the expected result?
The rc file should compile successfully.

What happens instead?
Compilation fails with the following error message:

rc: expected MENUITEM or POPUP, got SEPARATOR

Please use labels and text to provide additional information.
The MENUITEM SEPARATOR syntax should be supported as defined at https://msdn.microsoft.com/en-us/library/windows/desktop/aa381024(v=vs.85).aspx
 
Cc: thakis@chromium.org
The author of the .rc script is currently out of the office so any enhancements may take a while. Is there an in-progress change that requires this feature? If so then you may want to consider improving the script as part of that change.
@brucedawson: Thanks for the info. This is my first run of rc.py on an existing project so I'm filing bugs as I find them. As a workaround I'll avoid using "MENUITEM SEPARATOR" for the time being.
Yup, I didn't implement it since we didn't need it in chrome yet (https://github.com/nico/hack/blob/master/res/rc.cc#L21 https://github.com/nico/hack/blob/master/res/test/menu_opts.rc#L15 -- also see next line for a fun rc.exe quirk). Is this all you need or do you need more rc things?

Adding this should be fairly easy.
Added support: https://github.com/nico/hack/commit/706b05474ddb9ab90089966a81424e09f8775664

If you want this in chromium and are in a rush, you can run https://cs.chromium.org/chromium/src/build/toolchain/win/rc/upload_rc_binaries.sh?type=cs&sq=package:chromium to create a cl that picks up that change. Else I'll see if I find another five minutes for that.
Thanks! I have a workaround currently, so no rush.

> Is this all you need or do you need more rc things?

This is the only missing bit that I've run into so far.
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7

commit ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7
Author: Nico Weber <thakis@chromium.org>
Date: Thu Dec 07 18:37:10 2017

Update prebuilt rc binary.

CL created by running build/toolchain/win/rc/upload_rc_binaries.sh

Bug:  792594 , 792576 
Change-Id: I86fbfa2bbafed0670c0a7e2f43518b7a86d2c6b1
Reviewed-on: https://chromium-review.googlesource.com/813416
Reviewed-by: Marshall Greenblatt <marshall@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522484}
[modify] https://crrev.com/ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7/build/toolchain/win/rc/linux64/rc.sha1
[modify] https://crrev.com/ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7/build/toolchain/win/rc/mac/rc.sha1
[modify] https://crrev.com/ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7/build/toolchain/win/rc/win/rc.exe.sha1

Owner: thakis@chromium.org
Status: Fixed (was: Untriaged)
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 8 2017

Labels: merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40814bb06aba3d09b6a7556ce475e62693b905a4

commit 40814bb06aba3d09b6a7556ce475e62693b905a4
Author: Nico Weber <thakis@chromium.org>
Date: Fri Dec 08 20:02:43 2017

Update prebuilt rc binary.

CL created by running build/toolchain/win/rc/upload_rc_binaries.sh

Bug:  792594 , 792576 
Change-Id: I86fbfa2bbafed0670c0a7e2f43518b7a86d2c6b1
Reviewed-on: https://chromium-review.googlesource.com/813416
Reviewed-by: Marshall Greenblatt <marshall@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522484}(cherry picked from commit ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7)
Reviewed-on: https://chromium-review.googlesource.com/818094
Cr-Commit-Position: refs/branch-heads/3282@{#102}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/40814bb06aba3d09b6a7556ce475e62693b905a4/build/toolchain/win/rc/linux64/rc.sha1
[modify] https://crrev.com/40814bb06aba3d09b6a7556ce475e62693b905a4/build/toolchain/win/rc/mac/rc.sha1
[modify] https://crrev.com/40814bb06aba3d09b6a7556ce475e62693b905a4/build/toolchain/win/rc/win/rc.exe.sha1

Sign in to add a comment