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

Support filtering Items Arrays by Regex #458

Merged
merged 1 commit into from Aug 10, 2014

Conversation

@jugglinmike
Copy link
Contributor

@jugglinmike jugglinmike commented Jul 9, 2014

This is a convenience API for working with the @items array in nanoc's
preprocess block. In that context, it is common to manipulate items
according to a shared part of their identifier.

For example, with this change in place, users could succinctly create a
collection of all authors for a given site's weblog:

@items[%r{^/weblog/authors/}].each { |author|
  # etc...
}
@@ -34,6 +34,8 @@ def freeze
def [](*args)
if 1 == args.size && args.first.is_a?(String)
item_with_identifier(args.first)
elsif 1 == args.size && args.first.is_a?(Regexp)
@items.select { |i| i.identifier[args.first] }
Copy link
Member

@bobthecow bobthecow Jul 9, 2014

Choose a reason for hiding this comment

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

ewww. that's possibly the ugliest way of checking for a regex match :P

Loading

Copy link
Contributor Author

@jugglinmike jugglinmike Jul 9, 2014

Choose a reason for hiding this comment

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

I don't write nearly enough Ruby to know any better :P What would you recommend?

Loading

Copy link
Member

@bobthecow bobthecow Jul 9, 2014

Choose a reason for hiding this comment

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

i.identifier =~ args.first

Loading

Copy link
Contributor Author

@jugglinmike jugglinmike Jul 9, 2014

Choose a reason for hiding this comment

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

Ah, okay. Fixed.

I'm curious: is this purely stylistic or is there some performance consideration as well?

Loading

Copy link
Member

@bobthecow bobthecow Jul 10, 2014

Choose a reason for hiding this comment

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

I don't know, I've never benchmarked it.

rb_str_aref_m(int argc, VALUE *argv, VALUE str)
{
    if (argc == 2) {
        if (RB_TYPE_P(argv[0], T_REGEXP)) {
            return rb_str_subpat(str, argv[0], argv[1]);
        }
        return rb_str_substr(str, NUM2LONG(argv[0]), NUM2LONG(argv[1]));
    }
    rb_check_arity(argc, 1, 2);
    return rb_str_aref(str, argv[0]);
}

I imagine it's ever so slightly slower, since it has to check the number of arguments and check whether the first is a regexp before calling it, but mostly it's because it doesn't look right.

Loading

This is a convenience API for working with the `@items` array in nanoc's
`preprocess` block. In that context, it is common to manipulate items
according to a shared part of their identifier.

For example, with this change in place, users could succinctly create a
collection of all authors for a given site's weblog:

    @Items[%r{^/weblog/authors/}].each { |author|
      # etc...
    }
@coveralls
Copy link

@coveralls coveralls commented Jul 10, 2014

Coverage Status

Coverage increased (+0.71%) when pulling bb4a6c0 on jugglinmike:items-by-regex into db1db13 on nanoc:master.

Loading

@gpakosz
Copy link
Member

@gpakosz gpakosz commented Jul 22, 2014

LGTM

Loading

ddfreyne added a commit that referenced this issue Aug 10, 2014
Support filtering Items Arrays by Regex
@ddfreyne ddfreyne merged commit 93d5499 into nanoc:master Aug 10, 2014
1 check passed
Loading
@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Aug 10, 2014

Sweet!

Loading

@ddfreyne ddfreyne removed the to review label Mar 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants