Skip to content

refactor: add parseCommandLine()#7084

Merged
dlang-bot merged 2 commits intodlang:masterfrom
WalterBright:parseCommandLine
Aug 19, 2017
Merged

refactor: add parseCommandLine()#7084
dlang-bot merged 2 commits intodlang:masterfrom
WalterBright:parseCommandLine

Conversation

@WalterBright
Copy link
Member

This just pulls the code that loops over arguments[] into parseCommandLine(). You can't really tell that from the github diff, as github gets thoroughly confused.

This makes the inputs and outputs for parsing the command line clear, and removes the global variable references from it. All instances of global.params in parseCommandLine() are replaced with just params. It's much better encapsulated.

More can be done, but this is a good first approximation.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@JinShil
Copy link
Contributor

JinShil commented Aug 16, 2017

Since you're going through the work of refactoring this entire file, is it possible to start using D's string facilities instead of C's char * , strcmp, and friends?

@WalterBright
Copy link
Member Author

Yes, but my main concern is better encapsulation, rather than a tangle of dependencies.

@JinShil
Copy link
Contributor

JinShil commented Aug 17, 2017

If your main concern is a dependency on Phobos, I understand. But what about just druntime where string is defined. You can always add a string utilities module to DMD to avoid the dependency on Phobos.

@WalterBright
Copy link
Member Author

By encapsulation I mean having clear inputs and outputs. Dependency on Phobos is a separate issue.

Copy link
Member

@MartinNowak MartinNowak left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants

Comments