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

Extend Specification for CMake Commands #54

Closed
wants to merge 5 commits into from

Conversation

sanssecours
Copy link

@sanssecours sanssecours commented Jul 8, 2018

This PR extends the specification for some builtin CMake commands. It should improve the formatting for the commands

  • if
  • install,
  • export, and
  • write_basic_package_version_file

a little bit.

If I should change anything in this pull request, then please just comment below. Thank you.

@@ -511,11 +525,26 @@ def get_fn_spec():
"RUN_OUTPUT_VARIABLE": ZERO_OR_MORE
})

fn_spec.add(
Copy link
Owner

Choose a reason for hiding this comment

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

Hm... I wonder is it best to include functions that aren't builtin but provided by standard findscripts or listfiles? Lemme think about that...

fn_spec.add(
"if", flags=[], kwargs={
"NOT": ONE_OR_MORE,
"AND": ONE_OR_MORE,
"OR": ONE_OR_MORE
"OR": ONE_OR_MORE,
Copy link
Owner

Choose a reason for hiding this comment

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

I know its odd but looking at a few instances of complex boolean statements in my own listfiles, the formatting generally looks better if most of these act as "flags" even though they are really keywords. This is what I do in parser.py currently for conditional groups. It makes me think the formatting rules should be made more robust or configurable, but for now I'm going to do the same for the if spec.

@sanssecours sanssecours deleted the 💖 branch July 14, 2018 19:02
@cheshirekow
Copy link
Owner

cheshirekow commented Jul 14, 2018

I'm going to re-open this as v0.4.1 doesn't add write_basic_package_version_file. I need to decide what to do about that. One idea is to maybe just organize non-builtins in another module, and optionally include them when buliding the command spec?

Oh nevermind, original branch removed. Going to make an issue so I don't forget.

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.

2 participants