diff options
author | Jaxsun McCarthy Huggan | 2016-01-27 22:01:57 +0000 |
---|---|---|
committer | Jaxsun McCarthy Huggan | 2016-03-24 18:27:30 +0000 |
commit | 8980fe273e992b30019a1d6e014da5112c53adaf (patch) | |
tree | f8e2d6524a04f7d32f85cf6bcbe0f833af1ee68c /org.eclipse.mylyn.reviews.ui/src | |
parent | 4de0e3b0ba052151709d1d52ce52cb21417b9a28 (diff) | |
download | org.eclipse.mylyn.reviews-8980fe273e992b30019a1d6e014da5112c53adaf.tar.gz org.eclipse.mylyn.reviews-8980fe273e992b30019a1d6e014da5112c53adaf.tar.xz org.eclipse.mylyn.reviews-8980fe273e992b30019a1d6e014da5112c53adaf.zip |
488164: reviews editor displays inline comments as threads in patch sets
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=488164
Change-Id: I2432536f37f56403273e2bd51dcba06b26fb870e
Signed-off-by: Jaxsun McCarthy Huggan <jaxsun.mccarthy@tasktop.com>
Diffstat (limited to 'org.eclipse.mylyn.reviews.ui/src')
3 files changed, 292 insertions, 63 deletions
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java index 5c0a22580..065ea40bf 100644 --- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java +++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java @@ -51,7 +51,7 @@ import org.eclipse.swt.widgets.Display; /** * Base support for reviews labels. - * + * * @author Miles Parker * @author Steffen Pingel * @author Kevin Sawicki @@ -114,7 +114,6 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { } if (element instanceof IReviewItemSet) { return CommonImages.getImage(ReviewsImages.CHANGE_LOG); - } if (element instanceof IReviewItem) { IReviewItem item = (IReviewItem) element; @@ -156,6 +155,9 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { if (element instanceof GlobalCommentsNode) { return CommonImages.getImage(TasksUiImages.TASK); } + if (element instanceof ILocation) { + return CommonImages.getImage(ReviewsImages.REVIEW_QUOTE); + } return null; }; @@ -272,10 +274,11 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { return NLS.bind(Messages.ReviewsLabelProvider_X_Revision_Y, target.getPath(), target.getDescription()); } else if (target.getPath() == null && base.getPath() != null) { - return NLS.bind(Messages.ReviewsLabelProvider_X_Revision_Y, base.getPath(), target.getDescription()); + return NLS.bind(Messages.ReviewsLabelProvider_X_Revision_Y, base.getPath(), + target.getDescription()); } else if (!target.getPath().equals(base.getPath())) { - return NLS.bind(Messages.ReviewsLabelProvider_X_renamed_from_Y_Z, new Object[] { target.getPath(), - base.getPath(), target.getDescription() }); + return NLS.bind(Messages.ReviewsLabelProvider_X_renamed_from_Y_Z, + new Object[] { target.getPath(), base.getPath(), target.getDescription() }); } else { return NLS.bind(Messages.ReviewsLabelProvider_X_Revision_Y, target.getPath(), target.getDescription()); @@ -401,8 +404,8 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { }; }; - public static final TableColumnProvider SINGLE_COLUMN = new TableColumnProvider( - Messages.ReviewsLabelProvider_Items, 10, 120, false) { + public static final TableColumnProvider SINGLE_COLUMN = new TableColumnProvider(Messages.ReviewsLabelProvider_Items, + 10, 120, false) { @Override public Image getImage(Object element) { @@ -498,6 +501,8 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { } }; + private final boolean includeAuthors; + public static class Flat extends ReviewsLabelProvider { TableColumnProvider[] columns = new TableColumnProvider[] { ARTIFACT_COLUMN, COMMENTS_COLUMN, AUTHORS_COLUMN, DATE_COLUMN }; @@ -511,6 +516,14 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { public static class Tree extends ReviewsLabelProvider { TableColumnProvider[] columns = new TableColumnProvider[] { ITEMS_COLUMN, AUTHORS_COLUMN, DATE_COLUMN }; + public Tree() { + this(false); + } + + public Tree(boolean includeAuthors) { + super(includeAuthors); + } + @Override public TableColumnProvider[] getColumnProviders() { return columns; @@ -563,6 +576,14 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { } } + public ReviewsLabelProvider() { + this(false); + } + + public ReviewsLabelProvider(boolean includeAuthors) { + this.includeAuthors = includeAuthors; + } + @Override public String getText(Object element) { return ITEMS_COLUMN.getText(element); @@ -577,7 +598,11 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { if (element instanceof IComment) { styler = COMMENT_STYLE; } - StyledString styledString = new StyledString(getText(element), styler); + StyledString styledString = new StyledString(); + if (includeAuthors) { + addAuthorStyle(element, styledString); + } + styledString.append(getText(element), styler); if (element instanceof GlobalCommentsNode) { element = ((GlobalCommentsNode) element).getReview(); } @@ -594,6 +619,15 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider { return ITEMS_COLUMN.getImage(element); } + public static void addAuthorStyle(Object element, StyledString styledString) { + if (element instanceof IComment) { + IComment comment = (IComment) element; + String name = comment.getAuthor().getDisplayName(); + String firstName = StringUtils.substringBefore(name, " "); //$NON-NLS-1$ + styledString.append(firstName + ": ", AUTHOR_STYLE); //$NON-NLS-1$ + } + } + public static void addCommentLineNumberStyle(Object element, StyledString styledString) { if (element instanceof IComment) { IComment comment = (IComment) element; diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentProvider.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentProvider.java new file mode 100644 index 000000000..1b7af0c9c --- /dev/null +++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentProvider.java @@ -0,0 +1,131 @@ +package org.eclipse.mylyn.reviews.ui.spi.editor; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; + +import org.eclipse.jface.viewers.ITreeContentProvider; +import org.eclipse.jface.viewers.Viewer; +import org.eclipse.mylyn.reviews.core.model.IComment; +import org.eclipse.mylyn.reviews.core.model.IFileItem; +import org.eclipse.mylyn.reviews.core.model.ILocation; +import org.eclipse.mylyn.reviews.core.model.IReviewItemSet; + +import com.google.common.base.Function; +import com.google.common.base.Predicate; +import com.google.common.collect.FluentIterable; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Multimap; +import com.google.common.collect.Multimaps; + +final class ReviewSetContentProvider implements ITreeContentProvider { + + private final Multimap<ILocation, IComment> threads = LinkedHashMultimap.create(); + + private final Multimap<String, ILocation> threadLocationsByFile = LinkedHashMultimap.create(); + + public Object[] getElements(Object inputElement) { + return getReviewItems(inputElement).toArray(); + } + + @Override + public void inputChanged(Viewer viewer, Object oldInput, Object newInput) { + // do nothing + } + + @Override + public void dispose() { + threads.clear(); + threadLocationsByFile.clear(); + } + + @Override + public boolean hasChildren(Object element) { + return !getFileComments(element).isEmpty() || element instanceof ILocation; + } + + @Override + public Object getParent(Object element) { + // unsupported + return null; + } + + @Override + public Object[] getChildren(Object parentElement) { + if (parentElement instanceof IFileItem) { + IFileItem file = (IFileItem) parentElement; + Multimap<Long, IComment> commentsByLocation = Multimaps.index(getFileComments(file), commentLineNumber()); + + for (ILocation location : threadLocationsByFile.get(file.getId())) { + threads.removeAll(location); + } + threadLocationsByFile.removeAll(file.getId()); + + for (Long line : commentsByLocation.keySet()) { + ILocation location = null; + for (IComment comment : commentsByLocation.get(line)) { + if (location == null) { + location = comment.getLocations().iterator().next(); + threadLocationsByFile.put(file.getId(), location); + } + if (!threads.containsValue(comment)) { + threads.put(location, comment); + } + } + } + Collection<ILocation> relevantThreads = threadLocationsByFile.get(file.getId()); + ILocation[] locations = relevantThreads.toArray(new ILocation[relevantThreads.size()]); + Arrays.sort(locations, lineNumberComparator()); + return locations; + } else if (parentElement instanceof ILocation) { + return threads.get((ILocation) parentElement).toArray(); + } + return new Object[0]; + } + + private Comparator<ILocation> lineNumberComparator() { + return new Comparator<ILocation>() { + + @Override + public int compare(ILocation o1, ILocation o2) { + return (int) (o1.getIndex() - o2.getIndex()); + } + }; + } + + private Function<? super IComment, Long> commentLineNumber() { + return new Function<IComment, Long>() { + + @Override + public Long apply(IComment comment) { + return comment.getLocations().iterator().next().getIndex(); + } + }; + } + + private Predicate<IComment> hasLocation() { + return new Predicate<IComment>() { + + @Override + public boolean apply(IComment input) { + return input.getLocations().iterator().hasNext(); + } + }; + } + + private List<IFileItem> getReviewItems(Object inputElement) { + if (inputElement instanceof IReviewItemSet) { + return ((IReviewItemSet) inputElement).getItems(); + } + return ImmutableList.of(); + } + + private List<IComment> getFileComments(Object inputElement) { + if (inputElement instanceof IFileItem) { + return FluentIterable.from(((IFileItem) inputElement).getAllComments()).filter(hasLocation()).toList(); + } + return ImmutableList.of(); + } +}
\ No newline at end of file diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java index 807b0613c..5da4f5235 100644 --- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java +++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java @@ -14,9 +14,10 @@ package org.eclipse.mylyn.reviews.ui.spi.editor; import java.text.DateFormat; import java.util.ArrayList; -import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.eclipse.jface.layout.GridDataFactory; import org.eclipse.jface.layout.GridLayoutFactory; @@ -24,16 +25,18 @@ import org.eclipse.jface.layout.LayoutConstants; import org.eclipse.jface.viewers.ColumnViewerToolTipSupport; import org.eclipse.jface.viewers.DelegatingStyledCellLabelProvider; import org.eclipse.jface.viewers.IOpenListener; -import org.eclipse.jface.viewers.IStructuredContentProvider; -import org.eclipse.jface.viewers.IStructuredSelection; +import org.eclipse.jface.viewers.ITreeSelection; +import org.eclipse.jface.viewers.ITreeViewerListener; import org.eclipse.jface.viewers.OpenEvent; -import org.eclipse.jface.viewers.TableViewer; -import org.eclipse.jface.viewers.Viewer; +import org.eclipse.jface.viewers.TreeExpansionEvent; +import org.eclipse.jface.viewers.TreePath; +import org.eclipse.jface.viewers.TreeViewer; import org.eclipse.jface.window.ToolTip; import org.eclipse.mylyn.commons.ui.compatibility.CommonColors; import org.eclipse.mylyn.commons.workbench.forms.ScalingHyperlink; import org.eclipse.mylyn.internal.reviews.ui.providers.ReviewsLabelProvider; import org.eclipse.mylyn.internal.tasks.ui.editors.EditorUtil; +import org.eclipse.mylyn.reviews.core.model.IComment; import org.eclipse.mylyn.reviews.core.model.ICommit; import org.eclipse.mylyn.reviews.core.model.IFileItem; import org.eclipse.mylyn.reviews.core.model.IRepository; @@ -41,6 +44,7 @@ import org.eclipse.mylyn.reviews.core.model.IReview; import org.eclipse.mylyn.reviews.core.model.IReviewItemSet; import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer; import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfObserver; +import org.eclipse.mylyn.reviews.internal.core.ReviewFileCommentsMapper; import org.eclipse.mylyn.reviews.internal.core.TaskBuildStatusMapper; import org.eclipse.mylyn.tasks.core.data.TaskAttribute; import org.eclipse.mylyn.tasks.ui.editors.AbstractAttributeEditor; @@ -52,6 +56,7 @@ import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Text; +import org.eclipse.swt.widgets.TreeItem; import org.eclipse.ui.forms.FormColors; import org.eclipse.ui.forms.IFormColors; import org.eclipse.ui.forms.events.ExpansionAdapter; @@ -66,6 +71,7 @@ import org.eclipse.ui.forms.widgets.Section; * @author Miles Parker * @author Sam Davis */ +@SuppressWarnings("restriction") public class ReviewSetContentSection { private static final int MAXIMUM_ITEMS_SHOWN = 30; @@ -76,16 +82,14 @@ public class ReviewSetContentSection { private final Section section; - private TableViewer viewer; + private TreeViewer viewer; private final RemoteEmfObserver<IReviewItemSet, List<IFileItem>, String, Long> itemListObserver = new RemoteEmfObserver<IReviewItemSet, List<IFileItem>, String, Long>() { @Override public void updated(boolean modified) { createItemSetTable(); - if (modified) { - updateItemSetTable(); - } + updateItemSetTable(); updateMessage(); createButtons(); } @@ -96,7 +100,9 @@ public class ReviewSetContentSection { } }; - private Composite tableContainer; + private final RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, String, ?, ?, Long> itemSetConsumer; + + private Composite treeContainer; private Composite actionContainer; @@ -104,7 +110,8 @@ public class ReviewSetContentSection { private final AbstractTaskEditorPage page; - public ReviewSetContentSection(ReviewSetSection parentSection, final IReviewItemSet set, AbstractTaskEditorPage page) { + public ReviewSetContentSection(ReviewSetSection parentSection, final IReviewItemSet set, + AbstractTaskEditorPage page) { this.parentSection = parentSection; this.set = set; this.page = page; @@ -116,15 +123,14 @@ public class ReviewSetContentSection { section.setTitleBarForeground(parentSection.getToolkit().getColors().getColor(IFormColors.TITLE)); parentSection.addTextClient(parentSection.getToolkit(), section, "", false); //$NON-NLS-1$ - final RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, String, ?, ?, Long> itemSetConsumer = getParentSection().getReviewEditorPage() + itemSetConsumer = getParentSection().getReviewEditorPage() .getFactoryProvider() .getReviewItemSetContentFactory() .getConsumerForLocalKey(set, set.getId()); itemListObserver.setConsumer(itemSetConsumer); - final RemoteEmfConsumer<IRepository, IReview, String, ?, ?, Date> reviewConsumer = getParentSection().getReviewEditorPage() - .getFactoryProvider() - .getReviewFactory() - .getConsumerForModel(set.getReview().getRepository(), set.getReview()); + final RemoteEmfConsumer<IRepository, IReview, String, ?, ?, Date> reviewConsumer = getParentSection() + .getReviewEditorPage().getFactoryProvider().getReviewFactory().getConsumerForModel( + set.getReview().getRepository(), set.getReview()); reviewObserver = new RemoteEmfObserver<IRepository, IReview, String, Date>() { @Override public void updated(boolean modified) { @@ -149,8 +155,10 @@ public class ReviewSetContentSection { } } }); + createMainSection(); createItemSetTable(); + updateItemSetTable(); updateMessage(); } @@ -172,9 +180,8 @@ public class ReviewSetContentSection { message += " " + Messages.Reviews_RetrievingContents; //$NON-NLS-1$ } } else { - message = NLS.bind(Messages.Reviews_UpdateFailure_X, itemListObserver.getConsumer() - .getStatus() - .getMessage()); + message = NLS.bind(Messages.Reviews_UpdateFailure_X, + itemListObserver.getConsumer().getStatus().getMessage()); } AbstractReviewSection.appendMessage(getSection(), message); @@ -209,17 +216,17 @@ public class ReviewSetContentSection { getTaskEditorPage().getAttributeEditorToolkit().adapt(editor); } - tableContainer = new Composite(composite, SWT.NONE); - tableContainer.setBackground(Display.getCurrent().getSystemColor(SWT.COLOR_DARK_GRAY)); - GridDataFactory.fillDefaults().span(4, 1).grab(true, true).applyTo(tableContainer); - GridLayoutFactory.fillDefaults().numColumns(2).applyTo(tableContainer); + treeContainer = new Composite(composite, SWT.NONE); + treeContainer.setBackground(Display.getCurrent().getSystemColor(SWT.COLOR_DARK_GRAY)); + GridDataFactory.fillDefaults().span(4, 1).grab(true, true).applyTo(treeContainer); + GridLayoutFactory.fillDefaults().numColumns(2).applyTo(treeContainer); actionContainer = new Composite(composite, SWT.NONE); GridDataFactory.fillDefaults().span(4, 1).grab(true, true).applyTo(actionContainer); GridLayoutFactory.fillDefaults().numColumns(4).applyTo(actionContainer); createButtons(); - parentSection.getTaskEditorPage().reflow(); + reflow(); } private AbstractTaskEditorPage getTaskEditorPage() { @@ -338,36 +345,19 @@ public class ReviewSetContentSection { } else { style |= SWT.NO_SCROLL; } - viewer = new TableViewer(tableContainer, style); + viewer = new TreeViewer(treeContainer, style); GridDataFactory.fillDefaults() .span(2, 1) .grab(true, true) .hint(500, heightHint) .applyTo(viewer.getControl()); - viewer.setContentProvider(new IStructuredContentProvider() { - public void dispose() { - // ignore - } + viewer.setContentProvider(new ReviewSetContentProvider()); - public Object[] getElements(Object inputElement) { - return getReviewItems(inputElement).toArray(); - } - - private List<IFileItem> getReviewItems(Object inputElement) { - if (inputElement instanceof IReviewItemSet) { - return ((IReviewItemSet) inputElement).getItems(); - } - return Collections.emptyList(); - } - - public void inputChanged(final Viewer viewer, Object oldInput, Object newInput) { - } - }); ColumnViewerToolTipSupport.enableFor(viewer, ToolTip.NO_RECREATE); final DelegatingStyledCellLabelProvider styledLabelProvider = new DelegatingStyledCellLabelProvider( - new ReviewsLabelProvider.Simple()) { + new ReviewsLabelProvider.Tree(true)) { @Override public String getToolTipText(Object element) { //For some reason tooltips are not delegated.. @@ -377,24 +367,80 @@ public class ReviewSetContentSection { viewer.setLabelProvider(styledLabelProvider); viewer.addOpenListener(new IOpenListener() { public void open(OpenEvent event) { - IStructuredSelection selection = (IStructuredSelection) event.getSelection(); - IFileItem item = (IFileItem) selection.getFirstElement(); - if (item != null) { - getParentSection().getUiFactoryProvider() - .getOpenFileFactory(ReviewSetContentSection.this.getParentSection(), set, item) - .execute(); + // TODO: open to line of selected comment or thread + ITreeSelection selection = (ITreeSelection) event.getSelection(); + TreePath[] paths = selection.getPaths(); + for (TreePath path : paths) { + for (int i = 0; i < path.getSegmentCount(); i++) { + Object o = path.getSegment(i); + if (o instanceof IFileItem) { + IFileItem item = (IFileItem) o; + getParentSection().getUiFactoryProvider() + .getOpenFileFactory(ReviewSetContentSection.this.getParentSection(), set, item) + .execute(); + } + } } } }); - EditorUtil.addScrollListener(viewer.getTable()); + EditorUtil.addScrollListener(viewer.getTree()); viewer.setInput(set); - getParentSection().getTaskEditorPage().reflow(); + viewer.addTreeListener(new ITreeViewerListener() { + + @Override + public void treeExpanded(TreeExpansionEvent event) { + reflowAsync(); + } + + @Override + public void treeCollapsed(TreeExpansionEvent event) { + reflowAsync(); + } + }); } } - public void updateItemSetTable() { - if (set.getItems().size() > 0 && viewer != null) { + private void updateItemSetTable() { + if (set.getItems().size() > 0 && hasViewer()) { viewer.setInput(set); + TaskAttribute fileComments = ReviewSetContentSection.this.page.getModel() + .getTaskData() + .getRoot() + .getAttribute(ReviewFileCommentsMapper.FILE_ITEM_COMMENTS); + + final Set<IComment> toHighlight = new HashSet<IComment>(); + if (fileComments != null) { + List<IFileItem> files = itemSetConsumer.getModelObject(); + for (IFileItem file : files) { + for (IComment comment : file.getAllComments()) { + if (fileComments.getAttribute(comment.getId()) == null) { + toHighlight.add(comment); + } + } + } + } + + Display.getDefault().asyncExec(new Runnable() { + + @Override + public void run() { + if (hasViewer()) { + viewer.refresh(); + viewer.expandAll(); + decorateItems(viewer.getTree().getItems(), toHighlight); + reflow(); + } + } + }); + } + } + + private void decorateItems(TreeItem[] items, Set<IComment> toHighlight) { + for (TreeItem item : items) { + if (toHighlight.contains(item.getData())) { + item.setBackground(getTaskEditorPage().getAttributeEditorToolkit().getColorIncoming()); + } + decorateItems(item.getItems(), toHighlight); } } @@ -406,7 +452,7 @@ public class ReviewSetContentSection { getParentSection().getUiFactoryProvider().createControls(getParentSection(), actionContainer, getParentSection().getToolkit(), set); actionContainer.layout(); - getParentSection().getTaskEditorPage().reflow(); + reflow(); } } @@ -418,6 +464,24 @@ public class ReviewSetContentSection { return parentSection; } + private void reflowAsync() { + Display.getDefault().asyncExec(new Runnable() { + + @Override + public void run() { + reflow(); + } + }); + } + + private void reflow() { + getParentSection().getTaskEditorPage().reflow(); + } + + private boolean hasViewer() { + return viewer != null && !viewer.getControl().isDisposed(); + } + public void dispose() { itemListObserver.dispose(); reviewObserver.dispose(); |