Am 25.08.2016 um 09:25 schrieb Jiří Techet:<br>
>     This is specifically the type of function I want to avoid. This is<br>
>     not extensible at all, so not a good fit for a plugin API. Also,<br>
>     parameter-heavy functions are generally hard to use correctly (it's<br>
>     sometimes necessary but in this case my proposal gives a better<br>
>     solution IMO).<br>
><br>
> But yours isn't very consistent in this respect either - sorting<br>
> parameters are provided as arguments of tm_query_exec() while the rest<br>
> is provided by the query setters.<br>
<br>
<br>
It's a bit like with DB queries. Query filters (WHERE clause) apply to <br>
the query itself, post-processing (ORDER BY, GROUP BY) applies to the <br>
result set.<br>
<br>
Or did you mean to suggest that sorting/dedup should be set with a <br>
separate function? Well, I designed it such that these two don't affect <br>
the result itself, so that a query can re-run with different <br>
sorting/dedup. I guess you could also make _exec() parameter-less and <br>
export tm_tags_sort() and tm_tags_dedup() separately but this seems less <br>
convinient and makes tm_query interface look incomplete (because the two <br>
are in the tm_tags* namespace). But I guess I could live with it if <br>
necessary.<br>
<br>
><br>
> If you want to keep the setters instead of function parameters it's fine<br>
> - this wasn't the important thing in my comment. But I think it would be<br>
> much more flexible if it could be applied to an arbitrary tag array<br>
> instead of the hard-coded global or session tags. There's for instance<br>
> no way to get tags for the current document only. I would prefer<br>
><br>
> |GPtrArray *tm_query_filter(GPtrArray *tag_array, TMQuery *q,<br>
> TMTagAttrType *sort_attr, TMTagAttrType *dedup_attr) |<br>
><br>
> and it would filter the source tag_array whatever it is. In fact, I<br>
> would even prefer splitting this one into two:<br>
><br>
> |GPtrArray *tm_query_filter(GPtrArray *tag_array, TMQuery *q) |<br>
><br>
> and<br>
><br>
> |GPtrArray *tm_sort(GPtrArray *tag_array, TMTagAttrType *sort_attr,<br>
> TMTagAttrType *dedup_attr) |<br>
><br>
<br>
For my GI use case I must avoid referencing the `workspace->global_tags` <br>
and `workspace->tags_array`, because that's what's slowing down the <br>
python plugin (it seems to do stuff with getattr() for each element even <br>
if no python code actually accesses the elements). This is the main <br>
reason for the abstraction via `TM_QUERY_SOURCE_*`<br>
<br>
However, I am open to allow the query API to work also on local tag <br>
arrays. I could imagine a second constructor like this:<br>
|TMQuery *tm_query_new_from_tags(const TMWorkspace *workspace, TMTag <br>
**tags);|<br>
<br>
Then the remaining TMQuery interface could also be used on that kind of <br>
instance. The `workspace` parameter truly really needed but might become <br>
handy later on. `tags` would have to be suitably sorted by tag name though.<br>
<br>
For your idea to get tags for a specific document, I'd suggest a new filter.<br>
|gint tm_query_add_source_file(TMQuery *q, TMSourceFile *source_file);|<br>
<br>
Here shows the strength of the TMQuery interface. Constructors and <br>
filters can be added painlessly, without breaking existing users.<br>
<br>


<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/geany/geany/pull/1187#issuecomment-242358032">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABDrJ4avwgCAlZJZ670cVMiHcXe60zwYks5qjYASgaJpZM4JqVBL">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABDrJ0Bd8ZmvzqAMhAsChS7c2adhMAsQks5qjYASgaJpZM4JqVBL.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/geany/geany/pull/1187#issuecomment-242358032"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/geany/geany","title":"geany/geany","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/geany/geany"}},"updates":{"snippets":[{"icon":"PERSON","message":"@kugel- in #1187: Am 25.08.2016 um 09:25 schrieb Jiří Techet:\n\u003e     This is specifically the type of function I want to avoid. This is\n\u003e     not extensible at all, so not a good fit for a plugin API. Also,\n\u003e     parameter-heavy functions are generally hard to use correctly (it's\n\u003e     sometimes necessary but in this case my proposal gives a better\n\u003e     solution IMO).\n\u003e\n\u003e But yours isn't very consistent in this respect either - sorting\n\u003e parameters are provided as arguments of tm_query_exec() while the rest\n\u003e is provided by the query setters.\n\n\nIt's a bit like with DB queries. Query filters (WHERE clause) apply to \nthe query itself, post-processing (ORDER BY, GROUP BY) applies to the \nresult set.\n\nOr did you mean to suggest that sorting/dedup should be set with a \nseparate function? Well, I designed it such that these two don't affect \nthe result itself, so that a query can re-run with different \nsorting/dedup. I guess you could also make _exec() parameter-less and \nexport tm_tags_sort() and tm_tags_dedup() separately but this seems less \nconvinient and makes tm_query interface look incomplete (because the two \nare in the tm_tags* namespace). But I guess I could live with it if \nnecessary.\n\n\u003e\n\u003e If you want to keep the setters instead of function parameters it's fine\n\u003e - this wasn't the important thing in my comment. But I think it would be\n\u003e much more flexible if it could be applied to an arbitrary tag array\n\u003e instead of the hard-coded global or session tags. There's for instance\n\u003e no way to get tags for the current document only. I would prefer\n\u003e\n\u003e |GPtrArray *tm_query_filter(GPtrArray *tag_array, TMQuery *q,\n\u003e TMTagAttrType *sort_attr, TMTagAttrType *dedup_attr) |\n\u003e\n\u003e and it would filter the source tag_array whatever it is. In fact, I\n\u003e would even prefer splitting this one into two:\n\u003e\n\u003e |GPtrArray *tm_query_filter(GPtrArray *tag_array, TMQuery *q) |\n\u003e\n\u003e and\n\u003e\n\u003e |GPtrArray *tm_sort(GPtrArray *tag_array, TMTagAttrType *sort_attr,\n\u003e TMTagAttrType *dedup_attr) |\n\u003e\n\nFor my GI use case I must avoid referencing the `workspace-\u003eglobal_tags` \nand `workspace-\u003etags_array`, because that's what's slowing down the \npython plugin (it seems to do stuff with getattr() for each element even \nif no python code actually accesses the elements). This is the main \nreason for the abstraction via `TM_QUERY_SOURCE_*`\n\nHowever, I am open to allow the query API to work also on local tag \narrays. I could imagine a second constructor like this:\n|TMQuery *tm_query_new_from_tags(const TMWorkspace *workspace, TMTag \n**tags);|\n\nThen the remaining TMQuery interface could also be used on that kind of \ninstance. The `workspace` parameter truly really needed but might become \nhandy later on. `tags` would have to be suitably sorted by tag name though.\n\nFor your idea to get tags for a specific document, I'd suggest a new filter.\n|gint tm_query_add_source_file(TMQuery *q, TMSourceFile *source_file);|\n\nHere shows the strength of the TMQuery interface. Constructors and \nfilters can be added painlessly, without breaking existing users.\n\n"}],"action":{"name":"View Pull Request","url":"https://github.com/geany/geany/pull/1187#issuecomment-242358032"}}}</script>