Verify default compiler in help text matches given default compiler#337
Conversation
marler8997
requested review from
CyberShadow,
andralex and
wilzbach
as code owners
|
Thanks for your pull request and interest in making D better, @marler8997! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
rdmd_test.d
Outdated
| auto offset = helpText.indexOf(compilerHelpLine); | ||
| assert(offset >= 0); | ||
| auto compilerInHelp = helpText[offset + compilerHelpLine.length .. $]; | ||
| compilerInHelp = compilerInHelp[0 .. compilerInHelp.countUntil!isWhite]; |
There was a problem hiding this comment.
Don't mix slicing and countUntil! One counts code units, the other code points. It is always a mistake.
Either use byCodeUnit or just use indexOf (if we know the rest of the string, we should know what kind of whitespace it uses, too).
There was a problem hiding this comment.
So is countUntil performing autodecoding?
There was a problem hiding this comment.
Yes, countUntil operates on ranges, and strings are auto-decoded when interpreted as ranges.
There was a problem hiding this comment.
Also, do "end of line" sequences for WYSIWYG strings change or do they always match the source?
There was a problem hiding this comment.
Looking at dmd source, looks like they all normalize to \n
There was a problem hiding this comment.
Yes, countUntil operates on ranges, and strings are auto-decoded when interpreted as ranges.
How odd...this means that indexes into a "string range" will differ from indexes into a "string". I heard about this "autodecoding" confusion before but haven't run into it myself yet...illuminating
marler8997
force-pushed
the
rdmdTestVerifyDefaultCompiler
branch
from
2565753 to
0aeb69a
Compare
| auto compilerInHelp = helpText[offset + compilerHelpLine.length .. $]; | ||
| compilerInHelp = compilerInHelp[0 .. compilerInHelp.indexOf('\n')]; | ||
| assert(defaultCompiler.baseName == compilerInHelp); | ||
| } |
There was a problem hiding this comment.
This is a nice additional test, thanks for adding it.
However, if you're thinking that it provides a path to (in future) removing the need for the --rdmd-default-compiler flag ... it won't. See remarks in the earlier PR about why that is.
4 participants
No description provided.