-
Notifications
You must be signed in to change notification settings - Fork 444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding view::remove_when. #1219
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. You understood well.
/// \file | ||
// Range v3 library | ||
// | ||
// Copyright Eric Niebler 2013-present |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be your copyright. See #1137 (comment).
{ | ||
/// \addtogroup group-views | ||
/// @{ | ||
template<typename Rng, typename Pred> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these Pred
should be Fun
, like in split_when
, except for the one in the range adaptor that really takes a predicate and transforms it into a function.
range-v3/include/range/v3/view/split_when.hpp
Lines 44 to 45 in 66c9f5f
template<typename Rng, typename Fun> | |
struct split_when_view |
namespace detail | ||
{ | ||
template<typename Pred> | ||
struct predicate_pred |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be moved to the ../detail
directory, and definitely renamed, maybe to delimiter_specifier
.
|
||
namespace view | ||
{ | ||
/// Given a source range, unary predicate, and optional projection, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should go into doc/index.md
after remove_if
. I recommend the following description:
<DT>\link ranges::view::remove_when_fn `view::remove_when`\endlink</DT>
<DD>Given a source range and a delimiter specifier, filter out those elements specified by the delimiter specifier. The delimiter specifier can be a predicate or a function. The predicate should take a single argument of the range's reference type and return `true` if and only if the element is part of a delimiter. The function should accept an iterator and sentinel indicating the current position and end of the source range and return `std::make_pair(true, iterator_past_the_delimiter)` if the current position is a boundary; otherwise `std::make_pair(false, ignored_iterator_value)`.</DD>
test/view/remove_when.cpp
Outdated
@@ -0,0 +1,40 @@ | |||
// Range v3 library | |||
// | |||
// Copyright Eric Niebler 2014-present |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs rebase and conflict resolution.
static auto bind(remove_when_fn remove_when, Fun && fun) | ||
{ | ||
return make_pipeable(std::bind( | ||
remove_when, std::placeholders::_1, static_cast<Fun &&>(fun))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase on master and replace std::bind
with bind_back
, and make this function constexpr
. (You should also #include <range/v3/functional/bind_back.hpp>
.)
When you rebase on master, you also need to accommodate the Great Concept Rename (i.e., the great_concept_rename), and the fact that the |
Implements #1171, based on what my understanding is of what that's supposed to do.