Community
Participate
Working Groups
From Eugene: "Report URL" prefix is quite redundant. URL is speaking for itself...
So the format now will be: <prefix>: <description>, <url> I recall you flaming about multiline entries, and feel the same way. But I'm still wondering whether the URL is different enough that we put it on a new line, or simply put a comma in as above. I'd like the URL to remain non-optional in order for Mylar to encourage linking commit messages to reports.
That is exactly why I asked for templates. :-( Personally I'd prefer something like this (using an empty prefix for completed tsks): ---- ${prefix}${description} ${url} ---- There is not much use to show url in the comment shown in the table and it would look nicer on a new line when shown in comment text panel.
I've removed the URL prefix, and the URL is on a new line. However, I agree that the templates would be valuable so I'm leaving this open, but need to mark it helpwanted for now.
*** Bug 143673 has been marked as a duplicate of this bug. ***
Bumping up priority since there has been additional interest (bug 143673).
Multiline comments being good or bad aside (personally, I feel the convention should be left to be decided per project/organization managers), the question should rather be the choice of variables and allowing the users to specify their preferred format. I suggest following set of template variables for starters: ${task_title} - would expand to task title ${task_url} - would expand to task url ${last_comment} - for bugzilla tasks (see my explanation in bug 143673) I'd leave out prefix variable, as this is something that each template should define on their own... The default template would then probably look something like this: ---- <prefix>${task_title} ${task_url} ----
That looks good to me Roland. I hope that we can get to this for 0.6 because the current thing feels like a bit of a hack. But still keeping this helpwanted since this would be a nice self-contained contribution and there are a lot of high priority items on the table for 0.6.
I'm also very interested in this feature. It would be very helpful for EMFT projects to configure templates like: [${bug_id}] ${bug_summary} ... The EMFT build scripts can generate the release notes for me if commit comments contain lines that start with the bugzilla id in *square brackets*, but currently Mylar is not able to insert the closing bracket.
Is anyone interested in contributing this? It's a nicely decoupled feature and may be faciliated by Eclipse's templating mechanisms (although implementing the simple templating proposed here may be easier than figuring out those APIs).
Please make templates configurable per-repository. That probably going to need links to the projects too...
I'd give it a try, but unfortunately I've had trouble with setting up the developement environment for Mylar (Eating my own dogfood?) If anybody on the Mylar team could help me out, please drop me an email and maybe we'd be able to discuss the setting up of the Mylar dev environment in an online interactive fashion...
I'm very happy to help, and in general please don't be shy about posting questions like this to mylar-dev@eclipse.org. Any trouble that anyone has setting up the dev environment usually indicates that we need to simplify it or improve the docs. For now please try to follow these instructions: http://wiki.eclipse.org/index.php/Mylar_Contributor_Reference#Workspace_setup Post any trouble that you have either to this report or to mylar-dev, and if have trouble identifying the problem we can find a more interactive medium.
Created attachment 49080 [details] CommitTemplatesForMylar Patch Since always changing the commit comment drives me crazy I tried to develop a configurable templating mechanism. A patch for org.eclipse.mylar.team is attached. My approach is rather simple. Instead of specifying prefixes in the preferences, you can specify templates like the following: Completed task template: "[${task.id}] ${task.description} (COMPLETED)" Progress task template: "[${task.id}] ${task.description}" The template keywords are recognized by ITemplateHandler instances that are contributed to a simple new extension point like this: <extension point="org.eclipse.mylar.team.templateHandlers"> <templateHandler class="org.eclipse.mylar.internal.team.template.TaskCompletionDateTemplateHandler" description="Provides the completion date of a Mylar task." recognizedKeyword="task.completion.date"/> [...] There is currently no special UI for inserting the keyword tags into the templates on the preference page, because I'm not sure if you like this basic approach. What do you think?
Good stuff Eike. While it's more verbose, I like the "task.mumble" format because it is clear and will allow us flexibility (e.g. to use templates such as "repository.url" and "user.email"). I haven't applied the patch yet because there are conflicts on ..mylar.team/plugin.xml and ContextChangeSet, please try to recreate the patch. Also, there are a bunch of template handler classes that need to be created, but have very little functionality. What if you took your extension-point based approach a bit further and implemented this entirely as extension points. Rather than having the multiple handler classes you could have a single TaskPropertyMapper class that would take a string-based property and return the corresponding property from the task? The extension point could look something like the following, and TaskPropertyMapper would map the string-based property to the corresponding method on ITask. <taskCommentTemplate description="Provides the completion date of a Mylar task." property="task.completion.date"/> Also, CommitMessageTest should needs to be updated in order to ensure that our functionality of parsing commit messages to identify tasks should work. Otherwise we will not be able to map commit messages back to tasks (e.g. from Change Sets or the History view). The easiest way to ensure this works may be to require that task.id is in the comment after some identifiable string. Also, for any classes that you add please include a @author tag with your name so that credit goes where it is due.
Personally I'm not a fan of the idea to let the contribution do the parsing of the property keyword, only to reduce the amount of handler classes. It was my intent to enable the mylar.team plugin to decide which handler to use only with the information of the extension markup, so that the contributing plugins need only be loaded for really needed handlers. This is currently not implemented as such, but it could be done easily with the current approach. I've heard that nowadays inner classes can be used to instantiate contributions. This would reduce the amount of java source files (although the number of class files would even be increased by one). Shall I try that? When I synchronize my workspace right now there are no incoming changes at all. So I wonder where the conflicts you speak of do come from. Next I tend to have problems to use "Commit Context..." because it randomly complains that there are no interesting resources in the context. This occurs in my development workbench from time to time, but in my runtime workbench I can't get it to work at all, although there are changes in the Synchronize view. Can you help me with this issue?
Yes, we use inner classes to read contributions in a few places (e.g. see ContextCorePlugin). My main reason for suggesting not creating classes for each handler is not the bloat in the number of classes, but simplicity in creating new handlers. If they're entirely extension point based they're easier for others to make, and currently there doesn't seem to be a compelling reason for a custom handler class since each only has the effect of retrieving a property from ITask. Or did you have other handlers in mind? Regarding troubleshooting the context change sets, could you please take a look at the following, and then let me know if that does not resolve your problem so that we can diagnose further? http://wiki.eclipse.org/index.php/Mylar_FAQ#Change_Set_and_Synchronize_view_troubleshooting
I'm not sure what you mean by "entirely extension point based". Would you suggest to completely get rid of user contributed handler code? What would that extension point then be good for? I believe it's not the question of what I have in mind for additional handlers but *if* anybody would ever want to be able to tweak his or her own template logic. I can very well imagine such use cases where, for example by repository providers, special task information should be made available. Or one could use it to connect to organizational data via username, or... With the current approach this is easily feasible by only implementing a single method and providing markup for it. I'm currently refactoring the handler implementations into a single class. I'll post a new patch when some other changes a ready.
Yes, what I mean is to get rid of the handler classes, at least *until* there is a handler that needs them. Then we simply add the class attribute that you have for custom handling if there is a driver and use case for it. However, there has been a bit of a trend in Eclipse to make some functionality of this sort entirely extension point based since this can be more simple for cases where a lot of procedural logic is not needed, and so for this seems like such a case. But go ahead and submit your single handler class patch when that's done, and then we can take a closer look at what the better tradeoff is. I'm not opposed to the class-based approach.
Created attachment 49343 [details] Reworked patch (now with dynamic extensions and content assist)
Created attachment 49344 [details] mylar/context/zip
I have attached a reworked version of the commit templates. This version needs only one handler source file but can easily be extended with new handlers. The extension markup is not cached anymore, so that the TemplateHandlerManager is now aware of dynamic bundles and bundles are not activated unless really needed. To configure the templates on the preference page the user can now use content assist.
Comment on attachment 49343 [details] Reworked patch (now with dynamic extensions and content assist) incomplete
Comment on attachment 49344 [details] mylar/context/zip accidental
Created attachment 49347 [details] Reworked patch (now with dynamic extensions and content assist) This one should be complete. Sorry for inconveniences.
Created attachment 49369 [details] CommitTemplatesForMylar.patch (fixed an NPE and layout)
Created attachment 49380 [details] CommitTemplatesForMylar.patch (now with regex ID parser) I've tried to make the CommitMessageTest pass again. For this purpose I've modified ContextChangeSet.getTaskIdFromCommentOrLabel() so that it first tries to parse a template based comment (uses configurable regex)and in case of failure to parse a label-style value "id: ...". I think that all calls to getTaskIdFromCommentOrLabel() should be replaced by either getTaskIdFromComment() or getTaskIdFromLabel() to remove ambiguities. I leave this to you. While digging through your sources I was wondering why ITask has no getId() method. I have the feeling that the handling of task ids is rather fragile because it is spread over many places and heavily depends on varying contexts. I hope that my changes don't interfere with existing expactations. To have a nice content assist for the regex preference (id parser) I was forced to copy code from org.eclipse.ui.texteditor because it is not public. Please refer to org.eclipse.mylar.internal.team.ui.preferences.workaround.README.java for details.
Patch reviewed, not yet applied. Very nice work Eike, especially the content assist on the templates which will make it easy to use this. We should be on track on getting this contribution into 0.6.3, but will need the following: * Patches need to be formatted with our standard formatter, because currently it is impossible for me to tell which code changed in key classes like TeamPlugin (see http://wiki.eclipse.org/index.php/Mylar_Contributor_Reference#Contributing_patches for setting the formatter). * We'll need a test case or two for the new functionality, e.g. some tests that mock up a task (via new Task(..)), then create a comment from it given a predefined String that exercises the entire template, and then verifies that against the correct expansion of that template. This will ensure that future changes to the templates are covered. Additional coverage of parsing comments with different template formats would also be beneficial. * This may sound odd, but I'm concerned about the "regex to prase task id" being too complicated for all but advanced users. I think that you should still keep the regex support that you have underneath, but wonder if we could do a better job at hiding it from the user. How hard would it be to generate the regex from the template specifications?
Now that Eike has provided this flexibility, we need to come up with a good default. A similar topic came up on the platform-ui-dev@eclipse.org mailing list so I'll point them at this bug in case they have input. Eike, note that the frequent use of "[..]" for tagging descriptions on eclipse.org bugs makes that syntax a problematic choice for use in the template. How about this: ${task.status}: task ${task.id}\n${task.url} RESOLVED: bug 138666: [EditorMgmt] provide mechanism for restoring editors https://bugs.eclipse.org/bugs/show_bug.cgi?id=138666 This uses no special syntax other than ":", and puts the thing that changes the most first (i.e. the status), which is important because very little comment text fits onto a change set node in the Synchronize view. We need to think twice about the newline. I think that having the task URL in there is helpful for tools that render comments in the web browser, but noisy for reading. I've found that having the URL on a separate line makes it visually easier to ignore. Note that this template would allow us to merge the two templates into one.
One more decision point, we need to decide on one of the following for prefixing ${task.id}: 1) "bug 123: ": benefit is that this is consistent the Bugzilla web UI's and our task editor's hyperlinking of comments 2) "task 123: ": consistent with our naming and more generic for non-Bugzilla connectors 3) "123: ": most concise, doesn't impose naming but hyperlinking is less obvious
(In reply to comment #29) > One more decision point, we need to decide on one of the following for > prefixing ${task.id}: > 1) "bug 123: ": benefit is that this is consistent the Bugzilla web UI's and > our task editor's hyperlinking of comments > 2) "task 123: ": consistent with our naming and more generic for non-Bugzilla > connectors > 3) "123: ": most concise, doesn't impose naming but hyperlinking is less > obvious Mik, I'd say it should be connector-specific. So, use "bug 123" for Bugzilla, but other connector may choose not to att prefix. On the other hand if those templates are set per repository user could just make "bug" part of the template...
(In reply to comment #29) Now we see why it's important to have the "class" attribute in the extension point. If you decide to generate anything else than "123" for ${task.id} I will have to write my own handler that does so ;-) I did all the work only to see "[123]" at the beginning of the line because the EMFT releng scripts need this format to produce automatic release notes. I haven't tried this so far, but doesn't ${repository.type}-${task.id} produce something like "bugzilla-123"?
(In reply to comment #27) [...] > * Patches need to be formatted with our standard formatter, because currently > it is impossible for me to tell which code changed in key classes like > TeamPlugin (see > http://wiki.eclipse.org/index.php/Mylar_Contributor_Reference#Contributing_patches I can't find the file org.eclipse.mylar/developer/javaFormatterSettings.xml as refered to by the wiki. Oh, I juast realized, that Mylar has hidden it due to an empty task context and ctrl+shift+r didn't reveal it because the name has a typo. The folder "developer" contains a file named "javaFromatterSettings.xml".
... and in addition the misspelled file does not contain java code style formatter definitions. Seems to be more of a code template definition.
> Mik, I'd say it should be connector-specific. So, use "bug 123" for Bugzilla, > but other connector may choose not to att prefix. I agree. It may be a good idea to have a global default template that can be overridden per repository. For Trac we'll need to support: ${task.status}: ${task.description} (refs #${task.id})
Agreed the comments should be per-repository, created bug 157342 for that. Eike: apologies, for our formatter settings were overwritten. I've updated the wiki and they're now checked in as mylar-settings-formatter.xml. Because they're based on the Eclipse one and checked into the project settings you may just get the formatting automatically when you update. Are you willing to wrap up the points on comment#27? I appreciate if you can only dedicate enough time to this to get the format into that needed by EMFT, and am willing take the patch from here if needed, but it will take me a bit of time to get it integrated. What would be very helpful is if you could contribute the last point on comment#27. Considering Eike's suggestion on comment#31, I think that we should add a ${connector.task.prefix} variable, and have that expand to "bug" for Bugzilla and "refs #" for Trac. The slick thing about that is that we'll make it a part of the connector extension point so that they hyperlink detectors can use it. That would make the default: ${task.status}: ${connector.task.prefix} ${task.id}
Created attachment 50234 [details] CommitTemplatesPatch.txt (Now with RegexGuesser) Ok, I've tried to add a button that automatically guesses a regex to parse the task id. Seems to work. There's an autoGuess checkbox as well. Can't get the tests built since org.eclipse.mylar.core.tests is no longer in repo ;-( I'd suggest that we remove the possibility to enter 2 templates for progress and completed since this information can now be given by the template itself.
Excellent on the auto-regexp. Agreed on the suggestion to use one, less is more. The org.eclipse.mylar.core.tests project was renamed to org.eclipse.mylar.context.tests, so you just need to delete the old project and check that out. We maintain the up-to-date listing of the projects (via .psf files) here: http://wiki.eclipse.org/index.php/Mylar_Contributor_Reference#Workspace_setup I will plan to review and try to integrate this or an updated patch this afternoon/evening.
This is currently waiting on bug 145214, which is nearing completion, in order for us to avoid adding the full URL hyperlink into the default template. I would still like to aim for getting it into the next release (next Fridays' 0.7.0). Eike, should I assume you won't get a chance to provide a unit test?
Mik, I'm sorry but you're right. I'm the owner of the Net4j and CDO components of the EMFT project and I'm also quite busy with the preparations to align with Callisto 3.2.1. I'm managing a rather large code base and I'm my only project member ;-( This issue has already consumed much more time than I originally planned and even more than was necessary to fulfil my own functional requirements. Once I had at least your existing tests up and running (see attached patches) but unfortunately you decided to rename projects in the meantime. Please forgive me that I leave you without providing QA for my contribution and don't hesitate to contact me in the future if you have questions or concerns regarding my code.
Eike: please do not worry, it's great that in trying to make Mylar work for your process you made a contribution, and many others will benefit from this contribution. We systematically push people to contribute tests with patches, but we still take patches that come without test coverage because like you many others just need a couple of changes or improvements to get something working for them. I'm just a lot slower to apply those patches because we the fall into our regular prioritization, instead of just getting done when I'm doing bug triage. Btw, if you think they're at all reusable you could attach your tests as text to this bug report and I'll take it from there next week.
Unfortunately we ran out of time for getting this into 0.7.0. It should make it into the dev build week after next.
Done (finally!). Eike: I retained all of your contribution, but note that I modified it a fair amount in order to simplify the UI. I also did a revision on the naming, so it would be good if you could take a look over the changes. The most notable ones are: * There is only a single template instead of separate progress/completed templates. While communicating status is useful, this approach was problematic because it did not use connector-specific terms to indicate status. So instead I made the default template the following. In the case of Bugzilla ${connector.task.prefix} is "bug". ${task.status} - ${connector.task.prefix} ${task.id}: ${task.description} * The regexp used to match comments formatted in this way is always automatically determined based on this template in order to hide this implementation detail from the user. * URLs are not part of the default template and instead the association between a project and a task repository is used too look up the corresponding repository. Steffen: please check whether this is sufficient for you to get the format right for Trac. If the Trac format is sufficiently different you may also want to create a new bug for supporting different templates per-project. I'll cut a dev build with this functionality on Monday so that others can try it out before the 0.8 release planned for Friday.
Created attachment 52014 [details] mylar/context/zip
Mik, this is a really cool feature. It works perfectly for Trac. I think all that is needed now is the implementation of TracRepositoryUi.openRemoteTask() for opening tasks that are associated with commit sets from the synchronize view.
Great to hear. So are you going to need to implement AbstractRepositoryConnector.getTaskPrefix() for Trac? The default returns "task" and Bugzilla's returns "bug". There's an interesting thing about this prefix idea because the hyperlink detector should probably use it too. But we may want to support more than one prefix per-connector, and have the commit messages use the one considered to be default. If we go down that road we should probably declare the prefixes via extension point. Rob: CC'ing you since this is related to hyperlink detection. Steffen: regarding openRemoteTask(), yes, Trac should have because any time that some UI requests that a task be opened that is not in the task list that method is used. Create a bug for it and let us know if you need anything from the API to make it easier. Fyi, you'll know a task has been opened that way because it's editor icon only has the blue server overlay and no task icon.
Trac uses a hash sign '#' as the task prefix. I'll override the method and check if that works.
Mik, can you make bugzilla default like this? ${connector.task.prefix} ${task.id}: ${task.status} - ${task.description} It would look nicer in Sync view in changesets mode. I also wonder how can you handle task prefix for jira? It would be nice if empty prefix would work, but then Bugizlla's prefix frould include a trailing space...
Eugene: do you mean ${connector.task.prefix} ${task.id}: ${task.description} - ${task.status} ? If not, I worry that not having description after ID would look weird since that's such a common presentation. Also, the reason I proposed the status to be first is that it makes it easy to see the status changes of tasks (e.g. resolved) when scanning through incoming change sets. But I did have it in the last spot to start as you suggest. If you and others could try this out and comment more on the default that would be great. A dev build with this support is now available. Eugene: note that the connectors just specify "task#" or "bug", and the space comes from the template. Let's discuss Jira-specific requirements further on bug 150957.
I would prefer, if the task prefix would have the decision to include space character or not, since the specific connector should be aware of the exact prefix format (including or excluding space and all). As I understand, Trac uses hash symbol (#) as task prefix and although I have not used Trac myself I assume the full bug/task id looks something like "#1234" (not like "# 1234" - sans quotes). Thus it would make more sense to have the Also looking at how Bugzilla displays tooltips for bug links, I'd vote for having default template look like this: ${connector.task.prefix}${task.id}: ${task.status} - ${task.description}
(In reply to comment #48) > Eugene: do you mean > ${connector.task.prefix} ${task.id}: ${task.description} - ${task.status} > ? If not, I worry that not having description after ID would look weird since > that's such a common presentation. Because comment can be quite long and already look ugly in Sync view, putting status to the end does not make sense. I did meant to put it after the id, so sorting by comment text will bring issues nearby. > Eugene: note that the connectors just specify "task#" or "bug", and the space > comes from the template. Let's discuss Jira-specific requirements further on > bug 150957. As I said, if Jira would use empty prefix, that space in template will be somehow annoying. On the other hand, I've seen that some projects are using Jira pattern like [SPR-123], so it seems like we need a task suffix too... Though Jira issue ids are patternish on their own. That makes me wonder if there should be something like ${task.pattern} instead of ${connector.task.prefix} ${task.id}, so space would be inserted automatically where needed.
Reopening to address above comments.
I was playing with the latest dev-build, and I don't know why, the task.status variable was not substituted: ${task.status} - bug... blablabla I canceled the commit and noticed that in task list the bug was marked as unread. Then I double-clicked it, closed, commit again, and them the variable was substituted. I think there is a bug around here yet.
I've addressed most of these comments, including the bug, adding the space to the prefix, and making the text area multi-line. Key things to note: * I don't think we should put the status between the ID and the description. I tried and it looks weird and is inconsistent with the rest of the UI. If it's not suitable for the first position I think we either get rid of it or put it last (which is effectively getting rid of it). Thoughts? * For now Jira users will have to make that [..] pattern part of the template pattern. We'll probably need per-project templates before too long, i.e. bug 157342. A dev build with the fixes will be available soon, and I'll post to mylar-dev. Please try it out.
Created attachment 52413 [details] mylar/context/zip
One more thing: I pub back the ${task.url} by default on the newline. We don't require it anymore, but there has been people still rely on this (e.g. with email based commit messages), so the cost of the extra noise seems woth being 'backwards compatible' with tools that rely on URLs.
