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

Main: Default --doctool path to '.' if none given #47647

Merged
merged 1 commit into from
Apr 8, 2021

Conversation

akien-mga
Copy link
Member

Many users miss the . part of the documentation that indicates godot --doctool . as the usage, so let's make it the default (as it used to be years ago).

@akien-mga akien-mga added enhancement topic:core documentation cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Apr 5, 2021
@akien-mga akien-mga added this to the 4.0 milestone Apr 5, 2021
@akien-mga akien-mga requested a review from a team as a code owner April 5, 2021 13:37
@akien-mga akien-mga changed the title Main: Default --doctool path to '.' if none given Main: Default --doctool path to '.' if none given Apr 5, 2021
@akien-mga
Copy link
Member Author

akien-mga commented Apr 5, 2021

Note that this doesn't try to be overly clever so if you run godot --doctool --no-docbase or godot --doctool -v it will dump the stuff in folders named --no-docbase or -v. To specify arguments after --doctool <path>, the path is still mandatory.

Edit: Or actually no, with -v it seems to work fine, and with --no-docbase it gives:

ERROR: Argument supplied to --doctool must be a valid directory path.
   at: start (main/main.cpp:1916)

So seems fine.

main/main.cpp Outdated
Comment on lines 1885 to 1887
for (int j = i + 2; j < args.size(); j++) {
removal_docs.push_back(args[j]);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This was unused. No clue what it was supposed to do.

@YuriSizov
Copy link
Contributor

Otherwise it works as expected on Windows for me and I see no regressions if I want it to output somewhere else.

for (int j = i + 2; j < args.size(); j++) {
removal_docs.push_back(args[j]);
doc_tool_path = args[i + 1];
if (doc_tool_path.begins_with("-")) {
Copy link
Member Author

Choose a reason for hiding this comment

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

For the reference, this is something that could be done further up after else if (i < (args.size() - 1)) { for all "paired" parameters, but it's more risky so I'm keeping it localized.

The proper fix anyway is to scrap all this and redo the argument parsing with a proper parser (#44594).

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Not sure what --no-docbase is supposed to do exactly, but it did something at least. So yeah, it works on Windows as expected.

@akien-mga akien-mga merged commit 084b882 into godotengine:master Apr 8, 2021
@akien-mga akien-mga deleted the doctool-default-cwd branch April 8, 2021 12:12
@akien-mga
Copy link
Member Author

Cherry-picked for 3.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants