Skip to content

Rounded out command line switches and usage for compat with old cppwinrt.exe#164

Merged
Scottj1s merged 4 commits into
masterfrom
cmdline
Feb 8, 2019
Merged

Rounded out command line switches and usage for compat with old cppwinrt.exe#164
Scottj1s merged 4 commits into
masterfrom
cmdline

Conversation

@Scottj1s

@Scottj1s Scottj1s commented Feb 8, 2019

Copy link
Copy Markdown
Member

No description provided.

@Scottj1s Scottj1s requested a review from kennykerr as a code owner February 8, 2019 20:51
Comment thread src/tool/cpp/cppwinrt/code_writers.h Outdated
namespace xlang
{
static void write_license(writer& w)
static void write_prolog(writer& w)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe "preamble" rather than "prolog", not to be confused with https://en.wikipedia.org/wiki/Prolog 😉

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes good point

Comment thread src/tool/cpp/cppwinrt/main.cpp Outdated
{ "brackets", 0, 0 }, // Use angle brackets for #includes (defaults to quotes)
};

static void printUsage(writer& w, bool details)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The time you're spending with Java is rubbing off on you. 😉

print_usage

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, it is!

Comment thread src/tool/cpp/cppwinrt/main.cpp Outdated
1900 + tm.tm_year,
bind_each(printOption, options));

if (details)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think any of this needs to be in the tool. We can point to the docs page. Beyond that it seems impractical to try to make the tool self-documenting without substantially more effort. I think a list of options is sufficient and perhaps a link to the docs. Then we can focus on having quality detailed usage information in one place (online).

path Path to winmd file or recursively scanned folder
local Local ^%WinDir^%\System32\WinMetadata folder
sdk[+] Current version of Windows SDK [with extensions]
10.0.12345.0[+] Specific version of Windows SDK [with extensions]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like 10.0.12345.0 rather than a specific version. 👍

{ "help", 0, cmd::option::no_max, {}, "Show detailed help with examples" },
{ "root", 0, 1 }, // "Root folder name for projection headers (defaults to winrt)
{ "filter" }, // One or more prefixes to include in input (same as -include)
{ "lib", 0, 1 }, // Specify library prefix (defaults to winrt)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove "lib" - that was not in the internal version and doesn't work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

let's rip out the settings.component_lib stuff in a separate PR. for now it's undocumented.

{ "base", 0, 0, {}, "Generate base.h unconditionally" },
{ "opt", 0, 0, {}, "Generate component projection with unified construction support" },
{ "help", 0, cmd::option::no_max, {}, "Show detailed help with examples" },
{ "root", 0, 1 }, // "Root folder name for projection headers (defaults to winrt)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove "root" - that was not in the internal version and doesn't work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

also undocumented, so we can remove at leisure

struct usage_exception {};

static void process_args(int const argc, char** argv)
static std::vector<cmd::option> options

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've always wondered, why is this a static std::vector, instead of a constexpr array?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

great question

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because for some weird reason this doesn't work:

std::array<cmd::option, 18> options
{
    { "input", 0 },
    { "reference", 0 },
    { "output", 0, 1 },
    { "component", 0, 1 },

Instead, when I switch to using an array I need to do this:

std::array<cmd::option, 18> options
{
    cmd::option{ "input", 0 },
    cmd::option{ "reference", 0 },
    cmd::option{ "output", 0, 1 },
    cmd::option{ "component", 0, 1 },

Not a high priority so haven't dug into why exactly. Uniform initialization and all that... Feel free to fix whatever's wrong.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of std::array, the terse syntax should work with plain arrays:

constexpr cmd::option[] options
{
    { "input", 0},
    { "reference", 0},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pity.

@Scottj1s

Scottj1s commented Feb 8, 2019

Copy link
Copy Markdown
Member Author

No idea what happened with the PR - committing and will address any build breaks later.

@Scottj1s Scottj1s closed this Feb 8, 2019
@Scottj1s Scottj1s reopened this Feb 8, 2019
@Scottj1s Scottj1s closed this Feb 8, 2019
@Scottj1s Scottj1s reopened this Feb 8, 2019
@Scottj1s Scottj1s merged commit a82de4a into master Feb 8, 2019
@Scottj1s Scottj1s deleted the cmdline branch February 8, 2019 23:54
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