Community
Participate
Working Groups
On web interface, everywhere there is a hyperlink to a resolved bug, it appears in a strikethrought font. We need to do the same in bug editor hyperlinks, and in the dependency/blocks fields on attributes section.
Can we please make it configurable in appearance settings. Striketrough on small fonts is really hard to read.
BTW, this look like a duplicate of bug 191726 or the other way around.
*** This bug has been marked as a duplicate of bug 191726 ***
Reopening to address hyperlinks inside text, as stated in bug#191726 comment#22.
Steffen is it OK that I look to fix this? Or should we wait until bug 179245 is fixed?
I think we can address this bug independently of the editor refactoring. From a quick look at the code most of the changes required should contain TaskTextViewerConfiguration. The class will need a reference to the repository and set the text attributes based on task status. It will require a bit of additional parsing to extract the bug id but at best we would be able to leverage the hyperlink detection code for that. I have two concerns though: - Performance might be an issue: profiling indicated that tokenizing of comments is already impacting task editor startup time and additional task list lookups might make it noticeably slower. - Bug 200613 which suggests to allow connectors to contribute custom parsing rules is still unresolved and there could be some overlap with generalizing hyperlink detection and highlighting in the text viewers which would affect this bug. That said you are welcome to experiment with a fix but it might take some time and a few iterations until a good solution emerges. As a first pass you could try to limit changes to TaskTextViewerConfig and we could use that as driver for generalizing the parsing code (bug 200613).
Created attachment 87571 [details] mylyn/context/zip
Created attachment 87697 [details] Patch This Patch use StyleRange.strikeout for completed Tasks. I found some problems with the current implementation. If you look at an comment with more the one Hyperlink you find that in the WEB Interface all Hyperlinks are shown. But in Taskeditor we only activate one Hyperlink. Should we fix this?
Created attachment 87698 [details] mylyn/context/zip
Created attachment 87707 [details] screenshot That's a good start but it has some funny side effects (see screenshot). I noticed that the strikethrough decoration is only shown if you hover over a resolved task but maybe that's better than always showing it? Please file a new bug for the problem you were seeing with multiple hyperlinks and provide an example how to reproduce it.
Created attachment 87711 [details] corrected Patch Steffen, here is my correction. Please give this a try. Thanks.
Created attachment 87712 [details] mylyn/context/zip
Thanks Frank. I have committed your patch with minor performance optimizations. I'll keep this bug report open as a reminder for the remaining work mentioned in comment #6. Willian, could you test the if the strike-through works for you when you hover over links to resolved bugs?
Great sutff! Added note to the New & Noteworthy.
(In reply to comment #10) > That's a good start but it has some funny side effects (see screenshot). I > noticed that the strikethrough decoration is only shown if you hover over a > resolved task but maybe that's better than always showing it? I'd like to have it always shown, but it is a personal preference. (In reply to comment #13) > Willian, could you test the if the strike-through works for you when you hover > over links to resolved bugs? Works fine for me. There is a minor glitch, that the strikethrough only appears if the target task is present in task list. For example, bug#1 does not appear with the decoration here, because it is not captured in any of my queries, and consequently there is not a way to determine if it is resolved or not without making a network connection, I guess.
I have created bug 216694 to address that strikethrough only works for tasks in the task list.
(In reply to comment #6) > - Bug 200613 which suggests to allow connectors to contribute custom parsing > rules is still unresolved and there could be some overlap with generalizing > hyperlink detection and highlighting in the text viewers which would affect this > bug. > > That said you are welcome to experiment with a fix but it might take some time > and a few iterations until a good solution emerges. As a first pass you could > try to limit changes to TaskTextViewerConfig and we could use that as driver for > generalizing the parsing code (bug 200613). I start looking on this and have one point RepositoryTextScanner use rules for coloring hyperlinks but BugzillaConnector.findHyperlinks use a regexp Pattern. So by example the text 'some bug 123' is not colored but if you move the mouse over you see the hyperlink. Should I try do develop a new method which can be used from both points so that we can allow an overwrite later on.
Frank, it should be possible results from the hyperlink detectors to do the highlighting.
The idea is basically that once hyperlink detector is in place, you'll get the highlighting for free.
(In reply to comment #18) > Frank, it should be possible results from the hyperlink detectors to do the > highlighting. (In reply to comment #19) > The idea is basically that once hyperlink detector is in place, you'll get the > highlighting for free. Eugene, if iI see it rigtht there is a problem with this. hyperlink detector is only triggered by mouse move and highlighting is triggered by RepositoryTextScanner. This meens that hyperlink detector results are not available by the time RepositoryTextScanner needs this.
Frank, you can write an adapter for RepositoryTextScanner that would delegate to hyperlink detector. Hyperlink detector can be retrieved from connector ui.
Created attachment 88044 [details] 1. Prove of concept for comment#6 First version of custom parsing rules. Based on character scanner. TaskRule is an extention of SingleLineRule with Tasknumber detection between StartSequence and EndSequence. Next step: Change TaskRule to use a regexp for detection. Currently I use other colors for better visual feedback.
Created attachment 88045 [details] mylyn/context/zip
Created attachment 88078 [details] 2. Prove of concept for comment#6 TaskRule with regexp
Created attachment 88079 [details] mylyn/context/zip
Created attachment 88334 [details] new patch I use TaskRepository to define the regular expression infos. AbstractRepositoryConnectorUI could not be used because it get init after the editors are opend during startup.
Created attachment 88335 [details] mylyn/context/zip
There is another concern in regards to striketrough. Basically we can only show it if task data is available locally, so it is somewhat misleading to not show striketrough if task data is not available. So it may make sense to use different decoration or tooltip in case when we don't know if task is completed or not.
Great that you got the syntax highlighting working with regular expressions, Frank! I'll try to apply to my workspace in the next few days. Did you notice any slow down while browsing bugs or while entering new comments? I am not sure that the TaskRepository is the best place for storing the hyperlink expressions. Hyperlinks are a UI concern and should be part of the connector UI. The fact that it is not loaded when editors are restored presents a problem that we might have to solve first before we preceding with this bug. Rob and I briefly discussed an idea to delay editor initialization, e.g. display a message "Loading..." in the editor until the lazy startup in Tasks UI has completed and create the controls after that. I would suggest to fail gracefully for now, i.e. disable syntax highlighting for the editors that are restored on startup. TaskEditor.addPages() works the same way: If the connector UI is not there the task label (e.g. "Bug" or "Issue") is not displayed in the editor title. It only affects one task editor if it is the top most editor when the workbench is restored. Setting the priority back to P4 since this it is likely that this bug won't be resolved in time for 2.3 due to the technical challenges.
Created attachment 88674 [details] correction from comment#29 Following change: - AbstractRepositoryConnectorUi now used for the Method getHyperlinkRegularExpression - correction of the color I used for debug. Now I use ACTIVE_HYPERLINK_COLOR for normal bugs and HYPERLINK_COLOR for bugs not in tasklist. (In reply to comment #29) > Great that you got the syntax highlighting working with regular expressions, > Frank! I'll try to apply to my workspace in the next few days. Did you notice > any slow down while browsing bugs or while entering new comments? I know that it is slower, but I think that this is not so extreme that you can notice this. > I would suggest to fail gracefully for now, i.e. disable syntax highlighting for > the editors that are restored on startup. OK now the TaskRule is skipped if no connector UI is present. What do you think about this: In RepositoryTextScanner we can do the followning Set the parentEditor Message with a text like "Hyperlinkdetection is not present. Press synchronize button (right) to activate." But that meens we must pass the parentEditor from AbstractTaskEditorPage to RepositoryTextScanner.
Created attachment 88675 [details] mylyn/context/zip
(In reply to comment #30) > - AbstractRepositoryConnectorUi now used for the Method > getHyperlinkRegularExpression I would prefer to reuse the existing AbstractRepositoryConnectorUi.findHyperlinks() API instead of adding a new method. That way connector implementors would have a single place to specify the patterns and the API would not be tied to regular expressions. Would it work to pass the connectorUi object to TaskRule? > - correction of the color I used for debug. Now I use ACTIVE_HYPERLINK_COLOR for > normal bugs and HYPERLINK_COLOR for bugs not in tasklist. That sounds like a good idea. In addition we could add a tooltip, e.g. "Task not found in task list, click here to add". > In RepositoryTextScanner we can do the followning > > Set the parentEditor Message with a text like > "Hyperlinkdetection is not present. Press synchronize button (right) to > activate." That's a great idea. This should be done in TaskEditor as it affects other parts of the editor as well. We could make the message more generic, e.g.: "The editor may contain incomplete information. Click here to refresh."
(In reply to comment #32) > (In reply to comment #30) > > - AbstractRepositoryConnectorUi now used for the Method > > getHyperlinkRegularExpression > > I would prefer to reuse the existing > AbstractRepositoryConnectorUi.findHyperlinks() API instead of adding a new > method. That way connector implementors would have a single place to specify the > patterns and the API would not be tied to regular expressions. Would it work to > pass the connectorUi object to TaskRule? Currently getHyperlinkRegularExpression() is used in 1) BugzillaConnectorUi.findHyperlinks which returns TaskHyperlink[] 2) RepositoryTextScanner Constructor which only need the regular expressions. I don't see how we can easily get rid of getHyperlinkRegularExpression and use findHyperlinks instead. > That sounds like a good idea. In addition we could add a tooltip, e.g. "Task not > found in task list, click here to add". Please tell me how I can add a task to the unmatched container and I implement this. > That's a great idea. This should be done in TaskEditor as it affects other parts > of the editor as well. We could make the message more generic, e.g.: "The editor > may contain incomplete information. Click here to refresh." Implemented. Patch is comming soon.
Created attachment 88720 [details] Patch All thinks from comment #32 are now included. (In reply to comment #33) > > That sounds like a good idea. In addition we could add a tooltip, e.g. "Task not > > found in task list, click here to add". > > Please tell me how I can add a task to the unmatched container and I implement > this. I think I found a way. Please check if this is OK.
Created attachment 88721 [details] mylyn/context/zip
Created attachment 90296 [details] non working patch
I experimented with the patch for a bit but couldn't get it to work reliably. After looking at the RuleBasedScanner code more closely it seems that this will be hard to get working with regular expressions. The scanner goes through the text sequentially and delegates to the rules for each character that it scans until a matching token is found. The hyperlinks act more like scanners themselves by matching chunks of text against regular expressions. It might be worth investigating a custom implementation of ITokenScanner that is optimized to work with the hyperlink detectors. (In reply to comment #33) > Currently getHyperlinkRegularExpression() is used in > 1) BugzillaConnectorUi.findHyperlinks which returns TaskHyperlink[] > 2) RepositoryTextScanner Constructor which only need the regular expressions. > I don't see how we can easily get rid of getHyperlinkRegularExpression and use > findHyperlinks instead. I have attached part of the patch I was working on while experimenting with the scanner. It shows how to use findHyperlinks() from the token rule. > > That sounds like a good idea. In addition we could add a tooltip, e.g. "Task > not > > found in task list, click here to add". > > Please tell me how I can add a task to the unmatched container and I implement > this. I have opened bug 219745 to split this feature so we can get it into the current 2.3 release while iterating further on this bug.
(In reply to comment #37) > I experimented with the patch for a bit but couldn't get it to work reliably. > After looking at the RuleBasedScanner code more closely it seems that this will > be hard to get working with regular expressions. The scanner goes through the > text sequentially and delegates to the rules for each character that it scans > until a matching token is found. The hyperlinks act more like scanners > themselves by matching chunks of text against regular expressions. It might be > worth investigating a custom implementation of ITokenScanner that is optimized > to work with the hyperlink detectors. OK we can do an optimized ITokenScanner but what is with urlToken like SingleLineRule("http://", " ", urlToken)); What is possible is to do a specific ITokenScanner for Task and URL detection based on regaular expression for all cases. Should I do this? > > (In reply to comment #33) > > Currently getHyperlinkRegularExpression() is used in > > 1) BugzillaConnectorUi.findHyperlinks which returns TaskHyperlink[] > > 2) RepositoryTextScanner Constructor which only need the regular expressions. > > I don't see how we can easily get rid of getHyperlinkRegularExpression and use > > findHyperlinks instead. > > I have attached part of the patch I was working on while experimenting with the > scanner. It shows how to use findHyperlinks() from the token rule. Sorry I can not find this :-(
Created attachment 90418 [details] correction for non working patch
Created attachment 90419 [details] mylyn/context/zip
(In reply to comment #38) > OK we can do an optimized ITokenScanner but what is with urlToken like > SingleLineRule("http://", " ", urlToken)); > What is possible is to do a specific ITokenScanner for Task and URL detection > based on regular expression for all cases. > > Should I do this? It would be great if you could try to implement this. It should be possible to use regular expressions for URL detection as well as quoting (i.e. lines that start with '>'). Try the most simple implementation as a first pass to evaluate if this will satisfy performance requirements etc. Even if it doesn't scale for editing text it might still be feasible to do it for read-only comments. > > I have attached part of the patch I was working on while experimenting with > the > > scanner. It shows how to use findHyperlinks() from the token rule. > > Sorry I can not find this :-( Sorry, that was my bad. I attached the wrong patch. This is the code snippet I meant to attach: private static class TaskRule implements IPredicateRule { private IToken taskToken; private TaskRepository repository; private IToken completedTaskToken; private IToken taskNotFoundToken; private final AbstractRepositoryConnectorUi connectorUi; public TaskRule(TaskRepository repository, AbstractRepositoryConnectorUi connectorUi, IToken token, IToken tokenCompleted, IToken tokenNotFound) { this.repository = repository; this.connectorUi = connectorUi; this.taskToken = token; this.completedTaskToken = tokenCompleted; this.taskNotFoundToken = tokenNotFound; } public IToken evaluate(ICharacterScanner scanner, boolean resume) { String text = ((RepositoryTextScanner) scanner).getLineTextFromPos(); if (text == null || text.length() == 0) { return Token.UNDEFINED; } IHyperlink[] hyperlinks = connectorUi.findHyperlinks(repository, text, 0, 0); if (hyperlinks != null && hyperlinks.length > 0) { if (hyperlinks[0] instanceof TaskHyperlink) { TaskHyperlink taskHyperlink = (TaskHyperlink) hyperlinks[0]; TaskList taskList = TasksUiPlugin.getTaskListManager().getTaskList(); String repositoryUrl = taskHyperlink.getRepository().getUrl(); AbstractTask task = taskList.getTask(repositoryUrl, taskHyperlink.getTaskId()); if (task != null) { if (task.isCompleted()) { return completedTaskToken; } else { return taskToken; } } else { return taskNotFoundToken; } } } return Token.UNDEFINED; } public IToken getSuccessToken() { return taskToken; } public IToken evaluate(ICharacterScanner scanner) { return evaluate(scanner, false); } }
Created attachment 90658 [details] Patch with modified RuleBasedScanner It is OK to use a SingleLineRule with ColumnConstraint on Column 0 as quoteRule.
Created attachment 90659 [details] mylyn/context/zip
Another problem that is related to highlighting in the task editor (bug 146964 comment 30): Hyperlink coloring can be incorrect until the hyperlink is hovered over, at which point, the coloring is removed entirely. i.e. (bug #24) for testing :-) Note that this seems to only happen in the description.
Related: Part of the patch attached to bug 229014 has improvements for highlighting of hyperlinks.
David, this is remotely related to the WikiText integration. The goal is to decorate text in comments that is hyperlinked, such as bug references (e.g. bug 167941) or URLs. In addition bug references to resolved bugs should appear in strike through. The detection code is repository specific and usually implemented using regular expression (see findHyperlinks() in java class AbstractRepositoryConnectorUi). Do you have ideas how to implement a regular expression based ITokenScanner and how to integrate this with the existing parsing in wikitext?
The ITokenScanner could be implemented by delegating detection to the AbstractConnectorUi. It seems that the concrete implementations already use regex to find the hyperlinks. That said, I'm not sure if an ITokenScanner is the best way to go. We may be better off using document annotations and an annotation painter. This is what WikiText does for other types of hyperlinks. I'll take a look.
That sounds good. Can you point me at the classes in wikitext that handle the annotations? I would be interested in understanding how that works.
I'm still not sure which is the best approach. The Annotation+painter approach is probably simplest, but somewhat of a hack as you won't be altering the text presentation directly. This is how error/warning markers are painted (eg: the spell check). To understand how WikiText uses these you should take a look at the following: * for creating annotations: java class HtmlTextPresentationParser * for registering a custom annotation-based painter: java class HtmlViewer (see the @initPainter()@ method) Otherwise you'll have to alter the text presentation. For WikiText this is done differently depending on what's being parsed. * For an editable source viewer, see java class MarkupTokenScanner * For a read-only source viewer, see java class HtmlTextPresenter (not HTMLTextPresenter, as per bug 244352) * For viewers that use a @RuleBasedScanner@, you may just need to implement a rule. Unfortunately rules are extremely difficult to use with regex, since they use ICharacterScanner. In the past I've implemented a regex that works over ICharacterScanner, but it wasn't pretty. As you can see, it's not a trivial undertaking. The annotation-based stuff is definitely the simler path, as it should work with any viewer regardless of how the TextPresentation is calculated.
(In reply to comment #46) > David, this is remotely related to the WikiText integration. The goal is to > decorate text in comments that is hyperlinked, such as bug references (e.g. bug > 167941) or URLs. In addition bug references to resolved bugs should appear in > strike through. Other than strike through what kind of decoration did you have in mind? And should the decoration change after opening the editor if the status of the linked task changes?
It would be okay if the decoration was updated on editor refresh but if it could change when the task changes that would be even better. I can't think of a use case for making it change depending on whether a task is open or not. It would be nice if anything that is clickable would appear in a different color, e.g. blue. Without the WikiText extensions enabled this currently works for hyperlinks (e.g. http://eclipse.org) and tasks (e.g. 214529) but is hard coded in the java class RepositoryTextScanner at the moment.
Created attachment 110237 [details] patch that adds strikethrough style to task hyperlinks Patch does the following: * creates java class TaskHyperlinkTextPresentationManager, a class that can be used with any SourceViewer to manage task hyperlink styles * hooks TaskHyperlinkTextPresentationManager into ExtensibleRichTextAttributeEditor of the sandbox The patch works great, except that it depends on bug 244442. In my testing with a hacked fix for bug 244442 in BugzillaConnectorUi this patch works well. In my previous discussion of the solution I had forgotten about ITextPresentationListener. Of course, Eclipse APIs almost always has everything you need... this patch makes use of ITextPresentationListener, which is the intended way to add this kind of functionality.
Created attachment 110238 [details] mylyn/context/zip
Created attachment 110248 [details] re-cut the patch that adds strikethrough style to task hyperlinks seems that the previous cut of the patch was missing changes to a file.
That looks like a great approach that will provide a generic and powerful solution to tackle this problem! I'll apply the patch after bug 244442 is resolved.
Great stuff.
I have committed the patch but moved the code from the sandbox into tasks.ui. I noticed that the strikethrough only appears when I hover over the task for the first time. David, do you have any ideas?
Created attachment 113800 [details] patch that fixes the first-time-presentation issue (In reply to comment #57) > I have committed the patch but moved the code from the sandbox into tasks.ui. I > noticed that the strikethrough only appears when I hover over the task for the > first time. David, do you have any ideas? try this patch, it works for me. the problem is that the presentation was already computed before the manager was installed. This one-liner fixes the issue.
Thanks. Patch applied.
Marking resolved. Not that I made a small change to TaskHyperlinkTextPresentationManager.applyTextPresentation() to limit hyperlink detection to the "presentation's result window" instead of always scanning the whole text.
