Skip to content

Add missing message ID string, then refactor to make that harder to repeat#893

Merged
ychin merged 1 commit intomacvim-dev:masterfrom
s4y:missing-message-string
May 18, 2019
Merged

Add missing message ID string, then refactor to make that harder to repeat#893
ychin merged 1 commit intomacvim-dev:masterfrom
s4y:missing-message-string

Conversation

@s4y
Copy link
Copy Markdown
Contributor

@s4y s4y commented May 16, 2019

This PR contains a few commits:

  1. Adds "UseSelectionForFindMsgID" to MessageStrings: it was missing and the following strings were out of sync with the enum.
  2. Adds a macro to both declare the enum and create MessageStrings, to stop them from getting out of sync again. I don't have strong feelings about the details of how that's done; feel free to change it or suggest changes!
  3. Renames MessageStrings to MMVimMsgIDStrings, and makes it const, for clarity.

@eirnym
Copy link
Copy Markdown
Contributor

eirnym commented May 16, 2019

Looks good for me

@ychin
Copy link
Copy Markdown
Member

ychin commented May 17, 2019

Great catch! I imagine it must have been annoying with the debugging messages.

@s4y, can you do the following before I will merge:

  1. Fix the CI issues. You also need to modify MMBackend.mm. It's used in the Vim executable rather than the MacVim app which is why you probably didn't catch it but you should fail to build if you do a clean make.
  2. Squash the changes before I merge.

@s4y s4y changed the title Add missing message ID string, then refactor to make that harder to do again Add missing message ID string, then refactor to make that harder to repeat May 17, 2019
@s4y s4y force-pushed the missing-message-string branch from 063e870 to 239548c Compare May 17, 2019 15:49
@s4y
Copy link
Copy Markdown
Contributor Author

s4y commented May 17, 2019

Sure thing!

  1. Done. (Even a plain make catches it, but I leaned on Xcode's rename tool for this change and neglected to double check!)
  2. Done.

One type (`UseSelectionForFindMsgID`) was missing from `MessageStrings`,
which made the following strings out of sync with their IDs.

This change uses a macro to generate both, so they'll stay in sync, and
renames `MessageStrings` to `MMVimMsgIDStrings` to be more specific.
@s4y s4y force-pushed the missing-message-string branch from 239548c to 3caccbb Compare May 17, 2019 15:57
@ychin ychin merged commit 651ad8e into macvim-dev:master May 18, 2019
@s4y s4y deleted the missing-message-string branch May 18, 2019 17: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.

3 participants