Skip to content
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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

brevzin
Copy link
Collaborator

@brevzin brevzin commented Jun 19, 2019

Implements #1171, based on what my understanding is of what that's supposed to do.

Copy link
Contributor

@JohelEGP JohelEGP left a 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
Copy link
Contributor

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>
Copy link
Contributor

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.

template<typename Rng, typename Fun>
struct split_when_view

namespace detail
{
template<typename Pred>
struct predicate_pred
Copy link
Contributor

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,
Copy link
Contributor

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>

@@ -0,0 +1,40 @@
// Range v3 library
//
// Copyright Eric Niebler 2014-present
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Owner

@ericniebler ericniebler left a 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)));
Copy link
Owner

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>.)

@ericniebler
Copy link
Owner

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 view namespace is now the views namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants