acts-as-taggable-on
acts-as-taggable-on copied to clipboard
mbleigh
Provide prefix option in tagged_with method in tag.rb for prefix match.
Currently :wild supports substring match only and not specifically prefix match.
I will need specs for this feature. Update the changelog too to document this and squash the commits to have the code and the doc in the same one.
Thank you
@seuros yep, it's that one!
I think it could make sense to add also suffix, not only prefix :)
suffix and prefix are not good names IMO , i would prefer starting and ending. 'dog' is not a prefix of 'doggy', WDYT ?
@seuros I agree!
And what about something like: wild: :full, wild: :starting, wild: :ending or something like that?
Maybe it's not very elegant, but all both are wildcards, aren't they?
I don't like it. I want that coders understand the code without having to dig into the library to find it significance. Wild is ambiguous.
as scope , not in the wild option.
That way we could have .starting_with('c').ending_with('t')
Like the current PR .
not with wild: :full, wild: :starting_with, wild: :ending_with
Oooh yes, of curse :D I left that syntax after your last message ;) What I don't understand is: how do you want to implement starting_with with parameters?
User.tagged_with("my, tag, list", starting_with: ????) # => what is the parameter?
We can't do that with tagged_with
maybe
User.with_tags_starting_with('dog')
or
User.tagged.starting_with('cat')
I don't like it very much... :
In that way you can't do something like this:
User.tagged_with("my, list, of, tags", any: true, starting: true, order_by_matching_tag_count: true)
User.with_tags_starting_with(%w(my list of tags)).order_by_matching_tag_count
Mh, in that way it's pretty cool. For coherency should we change also tagged_with? Isn't it a breaking change?
We can't change it. We will just have to leave it as it and implement the new syntax.
We have to think about the context too.
User.tagged(['tags','skills']).starting_with('drive')
oh, interesting, I like it! Do you have an idea on how to implement this feature?
If we want to keep the actual structure we can have an internal QueryBuilder class:
class QueryBuilder
def initialize(parent)
@parent = parent
...
end
def order_by_matching_tag_count
@order << 'something_good'
self
end
def starting_with(string)
@where << 'something_good'
self
end
...
def to_a
query = @parent
query = @parent.select(select_clause.join(',')) unless @select_clause.empty?
query.joins(@joins.join(' '))
.where(@conditions.join(' AND '))
.group(@group)
.having(@having)
.order(@order_by.join(', '))
.readonly(false)
end
end
So you can do, in core.rb:
def tagged
QueryBuilder.new(self)
end
I don't know if it's a good idea, just sharing something I though. WDYT?
It should return a activerecord relation.
User.tagged.limit(3) User.tagged.where('xxx')
@seuros
do you ever tried to extend an active record relation? Me not.
Should it include this module?
https://github.com/rails/rails/blob/83e42d52e37a33682fcac856330fd5d06e5a529c/activerecord/lib/active_record/relation/query_methods.rb
@seuros Yes, I think so... I confess you I don't have a clear idea on how you like this to be done... Maybe I'm just taking this the wrong way.
Anyway, do you like to include those changes in this version or move everything in 4.0+?
Hi
Provide prefix option in tagged_with method Did we push the change for prefix match ?
Thanks
On Mon, Sep 1, 2014 at 12:44 PM, Piero Dotti [email protected] wrote:
@seuros https://github.com/seuros Yes, I think so... I confess you I don't have a clear idea on how you like this to be done... Maybe I'm just taking this the wrong way.
Anyway, do you like to include those changes in this version or move everything in 4.0+?
— Reply to this email directly or view it on GitHub https://github.com/mbleigh/acts-as-taggable-on/pull/568#issuecomment-54027798 .