Fix Issue 21722 - toString(sink, string format) does not work with non-"%s" strings

[berni44]

No description provided.

[dlang-bot]

Thanks for your pull request and interest in making D better, @berni44! 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: and Returns:)

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

Auto-close	Bugzilla	Severity	Description
✓	21722	major	toString(sink, string format) does not work with non-"%s" strings

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#7870"

[Geod24]

Thanks! However stable would have been better for a bugfix like this.

[berni44]

Thanks! However stable would have been better for a bugfix like this.

Huh, why? I thought, stable is only for regressions?

[Geod24]

Regressions and minor bug fixes. I think this would have qualified (and I sure would have loved to have it in the next LDC release). Another thing : The same bug exists with -preview=in and in parameters, see #7824 (which is a bugfix targeting stable).

[PetarKirov]

@berni44 even though currently the stable branch is short lived (master is merged into it every 2 months) it is better to think of it as an LTS branch, meaning it should be the default branch for all safe [*] bug fixes.

[*]: Of course, what is a safe bug fix requires judgement, and reasonable people may disagree, but you'd find that there's a category of bug fixes that generally everyone will agree are safe. For this particular case, I'd personally go for master, since it seems like it's enabling new functionality, but if it targeted stable, I wouldn't be against as well.

If stable was released more often (e.g. 2.096.1, 2.096.2, 2.096.3, 2.096.4, 2.096.5, 2.096.6, ..., 2.097.0) you would certainly see more push to include fixes there, as it would be the fastest way to deliver fixes to users. Currently, since many bug fixes still target master, users are forced to use master if they need those bug fixes, but as you know all deprecations, removals and in general any intentional or non-intentuonal breaking change happens there as well, so it's kind of risky in principle.

[berni44]

[*]: Of course, what is a safe bug fix requires judgement, and reasonable people may disagree, but you'd find that there's a category of bug fixes that generally everyone will agree are safe. For this particular case, I'd personally go for master, since it seems like it's enabling new functionality, but if it targeted stable, I wouldn't be against as well.

Hmm. Difficult. I have yet no clear picture, which fixes should go there, and which not... (Makes me feel like "don't fix anything, then you can't go wrong", which of course is not a good idea.)

breaking change happens there as well, so it's kind of risky in principle.

There is an other reason (although I didn't think of this yesterday, because it didn't occur to me, that stable might be a possible target), for targeting master in this case: Due to the split of std.format, I see a high risk, that this bugfix would be lost, when stable and master are merged (especially, when there are more than one of these, because in practice, git will only tell, that there is a conflict, showing a large bunch of code, that has been removed and an empty part, and @MartinNowak would be required to draw in all these changes once more manually); that's why I waited for the split to be finished, before going for bug fixes in std.format (though in my thoughts it involved two PRs targeting master and me who would have to do this manually, but it's the same problem).

[PetarKirov]

Hmm. Difficult. I have yet no clear picture, which fixes should go there, and which not... (Makes me feel like "don't fix anything, then you can't go wrong", which of course is not a good idea.)

If you're unsure where to put bug fix, you can always go for master, as all bug fixes eventually will be merged there. But I agree that we should have list of guidelines to make the decision making process more straightforward.

Due to the split of std.format, I see a high risk, that this bugfix would be lost [..]

Indeed that's a very good reason for targeting master for std.format bug fixes currently. Though I suppose after the first beta for 2.097.0, we can go back to normal.

[..] and MartinNowak would be required to draw in all these changes once more manually [..]

BTW, anyone can merge stable into master (e.g. using this script) and furthermore the best practice is to immediately open a PR to merge stable into master, after something was merged to stable, in order to keep the number of commits only present in stable as low as possible (hopefully zero).