Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Control of macro/function renaming #73

Closed
mikew-lunarg opened this issue Oct 1, 2018 · 5 comments
Closed

Feature request: Control of macro/function renaming #73

mikew-lunarg opened this issue Oct 1, 2018 · 5 comments
Assignees
Labels
feature-request Report requests some new capability or enhancement fix-in-flight

Comments

@mikew-lunarg
Copy link

Hi,
I have cmake-format v0.4.1 configured with command_case = u'lower'.

A side-effect is cmake-format is renaming invocations of CMake macros to lower-case, but not the definition of the macro. I haven't tried, but I'd assume similar happens to CMake functions.

Input:

macro(ACamelCaseMacro a b)
    message("hello")
endmacro()
ACamelCaseMacro(foo bar)

Output:

macro(ACamelCaseMacro a b)
    message("hello")
endmacro()
acamelcasemacro(foo bar)

I would prefer if cmake-format kept the formatting of macro names the same at both definition and invocation, and separate from command renaming; ideally there would be a separate config for macro and function renaming.

The reason this comes up is using camel-case for macros, so they may be quickly distinguished from native CMake commands.

A test case with input and output files is available here: https://github.com/mikew-lunarg/cmake_stuff/tree/macro_request/macro_request

Cheers
-- mew

@cheshirekow
Copy link
Owner

Hmm... To do this "correctly" would require implementing cmake. The formatter would have to know whether a statement was a macro or function, builtin or user defined. It would have to parse listfiles from the root up to the current one to insert these semantic differences. It'll be a long time before that is possible.

That said, one planned feature is per-command formatting options. You could specify case-preference on a per-command level. I could also add an option to format all unknown commands the same way. For your use case, you could format known commands as lower and unknown as keep.

Unfortunately it is not possible to distinguish a function from a macro from usage alone.

@mikew-lunarg
Copy link
Author

I was thinking along the lines of building a dictionary at each macro definition, then checking that dictionary to determine if the command was actually a macro. Although we try to keep the definition and invocation of project macros local to a file, I realize that's just style and not enforced.

Per-command formatting would probably do the job. I'm guessing there would be a global formatting option, with per-command settings as needed? That way I could set lower globally, with keep for the project's macros.

@cheshirekow cheshirekow added the acknowledged I've seen the report. I may not have anything more to say just yet! label Oct 2, 2018
@cheshirekow
Copy link
Owner

Although we try to keep the definition and invocation of project macros local to a file

I understand the temptation to take advantage of this convention, but I wouldn't say that it is a common convention and I don't think it's convention that I want to promote as something listfile authors "should do". I think implementing this kind of feature would lead to some unexpected and surprising behavior in the general case.

I'm guessing there would be a global formatting option, with per-command settings as needed?

Yeah, that's what I'm thinking. Maybe "per-command overrides" is a better name. I was originally thinking of it as a way to change the priority of the different formatting attempts for some "special" commands... but I think it could provide what you want.

I acknowledge that it is not ideal that you'll have to write your macros in one place, and then specify their formatting in an entirely separate place (cmake-format config file). Unfortunately I don't see a way around that without adding semantic understanding to cmake-format (currently, it only has syntactic understanding).

I think I would like to eventually add semantic understanding. I think it could get quite far by implementing a very small subset of cmake... but that's a very long term goal.

@cheshirekow cheshirekow added the feature-request Report requests some new capability or enhancement label Oct 2, 2018
@cheshirekow cheshirekow self-assigned this Oct 2, 2018
@mikew-lunarg
Copy link
Author

Thanks much, that seems workable (and definitely better than the manual process I'm using now to preserve the macro formatting)

Thanks again for a valuable tool!

@cheshirekow
Copy link
Owner

Implemented in 0.4.4 (to be pushed soon). Add a per_command config section. For instance cmake-format.py:

command_case = "lower"
per_command = {
  "mycommand" : {"command_case" : "unchanged"},
  "myothercommand" : {"command_case": "upper"},
}

and the behavior is like:
input:

NorMalComanD(foo bar baz)
MyCoMMaND(foo bar baz)
MyOtherCommand(foo bar baz)

output:

normalcomand(foo bar baz)
MyCoMMaND(foo bar baz)
MYOTHERCOMMAND(foo bar baz)

The mechanism for plumbing this override down to the formatter is a little... well... I'm kind of meh about it. I don't think it will work for some of the other configuration overrides I'm planning. If I change the plumbing in the future, the format of the override specification might change... but hopefully that wont be necessary.

@cheshirekow cheshirekow added fix-in-flight and removed acknowledged I've seen the report. I may not have anything more to say just yet! labels Dec 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Report requests some new capability or enhancement fix-in-flight
Projects
None yet
Development

No branches or pull requests

2 participants