dlang
Add optional parameter `appendNullTerminator` to `read` and `readText`
Thanks for your pull request and interest in making D better, @nordlow! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:
- My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
- My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
- I have provided a detailed rationale explaining my changes
- New or modified functions have Ddoc comments (with
Params:andReturns:)
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 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.
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "master + phobos#10856"
Hi @nordlow ,
Would be interesting to know so that we can evaluate adding this:
- What use case have you encountered that led you to propose this addition?
- Do you think it's a common enough use case to warrant adding it to the standard library? If so, why?
- Do any other programming languages or frameworks have a similar option in the standard library? If so, which? If not, why do you think D's standard library should do something inordinary?
@thewilsonator I think addition of new parameters which implement new features should be treated in the same way as addition of new symbols. Does that still need BDFL approval?
One way we can approach this is to just do this unconditionally; then, we don't need a new option to take up space in the argument list. It would mean allocating one extra byte when reading files, which is a negligible overhead. For empty files, we can return "" (the empty string literal) to avoid allocation.
One way we can approach this is to just do this unconditionally; then, we don't need a new option to take up space in the argument list. It would mean allocating one extra byte when reading files, which is a negligible overhead. For empty files, we can return
""(the empty string literal) to avoid allocation.
Agreed. Turns out the extra byte at the end is already allocated in the master version of readImpl. So then we just need to document this behavior and add a test verifying the null terminator at the end.
The motive for adding the null at the end is that it enables sentinel-based search in lexers. Being somewhat of a forgotten technique for increasing the speed of lexers and in turn parsers and scanners. Andrei Alexandrescu has talked about this previously on at least occation. I believe he used the term "sentinel-based search", IRC. FYI, @andralex.
The whitespace changes make it nigh impossible to find where the actual changes are, so I don't know what the diff should be. Having said, the title of the MR makes me question why this would be desired.
Having said, the title of the MR makes me question why this would be desired.
Did you read https://github.com/dlang/phobos/pull/10856#issuecomment-3263551675?
Having said, the title of the MR makes me question why this would be desired.
Did you read #10856 (comment)?
Yes, but why can't the user append it if they want to?
Having said, the title of the MR makes me question why this would be desired.
Did you read #10856 (comment)?
Yes, but why can't the user append it if they want to?
Can and should are two different things.
If you do it when you're dealing with buffers like read/readText should be, it can be free.
If the user does it outside of the buffer handling, that is very much not free.
