search

boostorg / algorithm

Boost.org algorithm module
http://boost.org/libs/algorithm
Boost Software License 1.0
114 stars 105 forks source link

use const_formatter insted of dissect_formatter Ticket #10131 #5

Closed ghost closed 5 years ago

ghost commented 10 years ago

reference to Ticket #10131

This may be a too naive solution to the problem I described in Ticket #10131, as I do not yet fully understand the concept behind the formatters.

This is my reasoning:

The trim_all___if() functions use the dissectformatter with the first element as the argument. Since we are actually just trimming *all I doubt that it is necessary to keep the "head". Since I found the trimfill*_if() functions with an empty replacement string to work correctly, we could use the same approach here. I've replaced the dissect_formatter with the const_formatter taking an empty string argument.

I have found this working with my code and examples. But as I said above, since I do not yet grasp the full concept of the formatters, there could be side effects I was not able to detect.

mclow commented 10 years ago

This is the wrong place to merge these changes. They should go into https://github.com/boostorg/algorithm:develop. Then, after the tests cycle (and we verify) that there's no problems on all platforms, we can merge to algorithm:master (which will get swept into boost::master)

That being said - thanks for doing this. The changes to the docs and to hex.hpp are uncontroversial, but the one to string/trim_all.hpp will require a bit of study. One thing - don't leave the old (incorrect code) in the file but commented out. Please remove them.

ghost commented 10 years ago

Am Mittwoch, 18. Juni 2014, 04.38:40 schrieb Marshall Clow:

This is the wrong place to merge these changes. They should go into https://github.com/boostorg/algorithm:develop. Then, after the tests cycle (and we verify) that there's no problems on all platforms, we can merge to algorithm:master (which will get swept into boost::master) Thanks for pointing this out. This makes sense and is pretty much what I wanted. I'm pretty new to the boost development process.

That being said - thanks for doing this. The changes to the docs and to hex.hpp are uncontroversial, but the one to string/trim_all.hpp will require a bit of study. One thing - don't leave the old (incorrect code) in the file but commented out. Please remove them. Will do so.

regards Willi ...

mclow commented 10 years ago

Checking the develop branch, it appears that the first two fixes have already been made; leaving only the one for the bug that you filed.

zamazan4ik commented 7 years ago

@mclow what is the current status of this PR?

mclow commented 7 years ago

this PR will never be merged - because it is against the master branch, not develop. However, there are a couple of things that I here might want to cherry-pick into the develop branch.

zamazan4ik commented 7 years ago

What do you want to cherry-pick to the develop branch from this PR? Maybe i can prepare valid PR to the develop branch (or maybe will be better if you will do it).

mclow commented 7 years ago

I'll do it.

zamazan4ik commented 7 years ago

Did you cherry-pick things from the PR?

mclow commented 5 years ago

The docs and the hex change have been merged. The change to trim broke a bunch of tests, so I'm not going to merge that.