Bug 336149

Comment 1 Benjamin Muskalla 2011-02-10 09:10:59 EST

Nathan, we'd need API for retrieving the tags of each node. I think having them listed on the marketplace/node itself besides categories, etc would make most sense. 	

Example: http://marketplace.eclipse.org/content/subclipse/api/p
New xml fragment:
<tags>
  <tag>scm</tag>
  <tag>svn</tag>
  <tag>version control</tag>
  ...
</tags>

Comment 2 Nathan Gervais 2011-02-10 09:57:08 EST

Sounds good.

I'll be keeping the same format for them as categories.

<tags>
  <tag id="##" name="tag_name"/>
<tag id="##" name="tag_name"/>

Comment 3 Nathan Gervais 2011-02-10 12:54:20 EST

http://marketplace.eclipse.org/content/subclipse/api/p

As requested.

Comment 4 Benjamin Muskalla 2011-02-10 15:07:50 EST

Wow, that was quick. Thanks Nathan. I'm currently working on the client side implementation and will provide a first patch tomorrow.

Comment 5 Benjamin Muskalla 2011-02-14 12:22:37 EST

Created attachment 188919 [details]
first draft

Here is a first draft of this functionality. Please do not commit until we agree on the UI part of this. Will rebase once source code is formatted (task 337145).

Comment 6 Benjamin Muskalla 2011-02-15 12:29:45 EST

Created attachment 189021 [details]
final patch

David, can we have a look at this together?

Comment 7 David Green 2011-02-15 16:50:09 EST

Benjamin, the patch looks great.  A couple of things to look at:
# the contentType event should be fired after setting the content type, not before
# the contentType event should be fired whenever the contentType changes, not just when doing a query for tags
# the contentType listener should look at the content type to determine whether or not to switch tabs
# consider making "contentType" a constant of MarketplaceViewer
# MarketplaceViewer.listeners should either be a LinkedHashSet or a List (it should preserve order)
# how does a tag search differ from a regular search?
# consider adding yourself as an @author
Let me know if you want to discuss

Comment 8 Benjamin Muskalla 2011-02-17 06:35:28 EST

Created attachment 189166 [details]
patch

> 1. the contentType event should be fired after setting the content type, not before
Makes sense, done
> 2. the contentType event should be fired whenever the contentType changes, not just when doing a query for tags
Done

> 3. the contentType listener should look at the content type to determine whether or not to switch tabs
Done

> 4. consider making "contentType" a constant of MarketplaceViewer
Done

> 5. MarketplaceViewer.listeners should either be a LinkedHashSet or a List (it should preserve order)
Done

> 6. how does a tag search differ from a regular search?
No difference here as stated in the task description. We're currenlty using the regular search for tags.

> 7. consider adding yourself as an @author
Done

Comment 9 David Green 2011-02-17 22:12:26 EST

Great work Benjamin.  It seems that the patch exceeds the guidelines for < 250 lines required for a CQ, however when looking at the patch most of the changes are to whitespace.  I wonder if it makes sense to reformat all of the source code, then re-cut the patch (without unnecessary whitespace changes)

Comment 10 David Green 2011-02-18 17:40:09 EST

I'll apply the patch and just not apply whitespace changes.

Comment 11 David Green 2011-02-18 19:36:58 EST

Thanks Benjamin!  Changes applied

Comment 12 David Green 2011-02-18 19:37:03 EST

Created attachment 189335 [details]
mylyn/context/zip

Comment 13 Carsten Reckord 2015-12-16 13:52:51 EST

Cleanup: closing all released fixes.