[vcs-log] IDEA-125276 (review IDEA-CR-4340) fixups for filtering branches by regexp

* rename isShown -> matches
* add javadoc
* remove final
* createFilter does not return null, check if values are empty outside
* rewrite toString in branch filter
* use GitBranchUtil.convertBranchesToNames for getting branch names for filtering log
* remove getSingleFilteredBranch method from API
* check whether the string is a regexp or just a weird branch by looking it up in existing branches names
This commit is contained in:
Julia Beliaeva
2015-11-03 19:25:15 +03:00
committed by Julia Beliaeva
parent 9fcdb59352
commit 43be34271d
13 changed files with 147 additions and 83 deletions
@@ -16,20 +16,26 @@
package com.intellij.vcs.log;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collection;
import java.util.regex.Pattern;
/**
* Tells to filter by branches with given names.
*/
public interface VcsLogBranchFilter extends VcsLogFilter {
boolean isShown(@NotNull String name);
@Nullable
String getSingleFilteredBranch();
/**
* Tells if a branch matches the filter.
*
* @param name branch name.
* @return true if a branch matches the filter, false otherwise.
*/
boolean matches(@NotNull String name);
/**
* Text presentation for the filter (to display in filter popup).
*
* @return text presentation for the filter.
*/
@NotNull
Collection<String> getTextPresentation();
}
@@ -1,5 +1,6 @@
package com.intellij.vcs.log.data;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.util.Function;
import com.intellij.util.containers.ContainerUtil;
@@ -7,13 +8,13 @@ import com.intellij.vcs.log.VcsLogBranchFilter;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.*;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
public class VcsLogBranchFilterImpl implements VcsLogBranchFilter {
private static final Logger LOG = Logger.getInstance(VcsLogBranchFilterImpl.class);
@NotNull private final List<String> myBranches;
@NotNull private final List<Pattern> myPatterns;
@@ -31,8 +32,7 @@ public class VcsLogBranchFilterImpl implements VcsLogBranchFilter {
}
@Deprecated
public VcsLogBranchFilterImpl(@NotNull Collection<String> branches,
@NotNull Collection<String> excludedBranches) {
public VcsLogBranchFilterImpl(@NotNull Collection<String> branches, @NotNull Collection<String> excludedBranches) {
myBranches = new ArrayList<String>(branches);
myPatterns = new ArrayList<Pattern>();
myExcludedBranches = new ArrayList<String>(excludedBranches);
@@ -41,32 +41,40 @@ public class VcsLogBranchFilterImpl implements VcsLogBranchFilter {
@Nullable
public static VcsLogBranchFilterImpl fromBranch(@NotNull final String branchName) {
return new VcsLogBranchFilterImpl(Collections.singletonList(branchName),
Collections.<Pattern>emptyList(),
Collections.<String>emptyList(),
Collections.<Pattern>emptyList());
return new VcsLogBranchFilterImpl(Collections.singletonList(branchName), Collections.<Pattern>emptyList(),
Collections.<String>emptyList(), Collections.<Pattern>emptyList());
}
@Nullable
public static VcsLogBranchFilterImpl fromTextPresentation(@NotNull final Collection<String> strings) {
if (strings.isEmpty()) return null;
List<String> branches = new ArrayList<String>();
@NotNull
public static VcsLogBranchFilterImpl fromTextPresentation(@NotNull Collection<String> strings, @NotNull Set<String> existingBranches) {
List<String> branchNames = new ArrayList<String>();
List<String> excludedBranches = new ArrayList<String>();
List<Pattern> patterns = new ArrayList<Pattern>();
List<Pattern> excludedPatterns = new ArrayList<Pattern>();
for (String string : strings) {
boolean isRegexp = isRegexp(string);
boolean isExcluded = string.startsWith("-");
string = isExcluded ? string.substring(1) : string;
boolean isRegexp = !existingBranches.contains(string);
if (isRegexp) {
if (isExcluded) {
excludedPatterns.add(Pattern.compile(string));
try {
Pattern pattern = Pattern.compile(string);
if (isExcluded) {
excludedPatterns.add(pattern);
}
else {
patterns.add(pattern);
}
}
else {
patterns.add(Pattern.compile(string));
catch (PatternSyntaxException e) {
LOG.warn("Pattern " + string + " is not a proper regular expression and no branch can be found with that name.", e);
if (isExcluded) {
excludedBranches.add(string);
}
else {
branchNames.add(string);
}
}
}
else {
@@ -74,12 +82,12 @@ public class VcsLogBranchFilterImpl implements VcsLogBranchFilter {
excludedBranches.add(string);
}
else {
branches.add(string);
branchNames.add(string);
}
}
}
return new VcsLogBranchFilterImpl(branches, patterns, excludedBranches, excludedPatterns);
return new VcsLogBranchFilterImpl(branchNames, patterns, excludedBranches, excludedPatterns);
}
@NotNull
@@ -113,11 +121,27 @@ public class VcsLogBranchFilterImpl implements VcsLogBranchFilter {
@Override
public String toString() {
return "on patterns: " + StringUtil.join(myPatterns, ", ") + "; branches: " + StringUtil.join(myBranches, ", ");
String result = "";
if (!myPatterns.isEmpty()) {
result += "on patterns: " + StringUtil.join(myPatterns, ", ");
}
if (!myBranches.isEmpty()) {
if (!result.isEmpty()) result += "; ";
result += "on branches: " + StringUtil.join(myBranches, ", ");
}
if (!myExcludedPatterns.isEmpty()) {
if (result.isEmpty()) result += "; ";
result += "not on patterns: " + StringUtil.join(myExcludedPatterns, ", ");
}
if (!myExcludedBranches.isEmpty()) {
if (result.isEmpty()) result += "; ";
result += "not on branches: " + StringUtil.join(myExcludedBranches, ", ");
}
return result;
}
@Override
public boolean isShown(@NotNull String name) {
public boolean matches(@NotNull String name) {
return isIncluded(name) && !isExcluded(name);
}
@@ -137,17 +161,4 @@ public class VcsLogBranchFilterImpl implements VcsLogBranchFilter {
}
return false;
}
@Nullable
@Override
public String getSingleFilteredBranch() {
if (!myPatterns.isEmpty()) return null;
if (myBranches.size() != 1) return null;
String branch = myBranches.get(0);
return isExcluded(branch) ? null : branch;
}
private static boolean isRegexp(@NotNull String pattern) {
return StringUtil.containsAnyChar(pattern, "()[]{}.*?+^$\\|");
}
}
@@ -147,7 +147,7 @@ class VisiblePackBuilder {
return new HashSet<Integer>(ContainerUtil.mapNotNull(refs.getBranches(), new Function<VcsRef, Integer>() {
@Override
public Integer fun(@NotNull VcsRef ref) {
boolean acceptRef = filter.isShown(ref.getName());
boolean acceptRef = filter.matches(ref.getName());
return acceptRef ? myHashMap.getCommitIndex(ref.getCommitHash()) : null;
}
}));
@@ -19,6 +19,7 @@ import com.intellij.openapi.util.Condition;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vfs.VfsUtilCore;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.util.Function;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.MultiMap;
import com.intellij.vcs.log.*;
@@ -157,4 +158,29 @@ public class VcsLogUtil {
}
});
}
@Nullable
public static String getSingleFilteredBranch(@NotNull VcsLogBranchFilter filter,
@NotNull VcsLogRefs refs,
@NotNull Set<VirtualFile> vcsRoots) {
String branchName = null;
Set<VirtualFile> checkedRoots = ContainerUtil.newHashSet();
for (VcsRef branch : refs.getBranches()) {
if (!filter.matches(branch.getName())) continue;
if (branchName == null) {
branchName = branch.getName();
}
else if (!branch.getName().equals(branchName)) {
return null;
}
if (checkedRoots.contains(branch.getRoot())) return null;
checkedRoots.add(branch.getRoot());
}
if (!checkedRoots.equals(vcsRoots)) return null;
return branchName;
}
}
@@ -21,7 +21,10 @@ import com.intellij.vcs.log.*;
import com.intellij.vcs.log.data.LoadingDetails;
import com.intellij.vcs.log.data.VcsLogDataHolder;
import com.intellij.vcs.log.data.VcsLogUiProperties;
import com.intellij.vcs.log.impl.VcsLogUtil;
import com.intellij.vcs.log.ui.filter.VcsLogClassicFilterUi;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.awt.*;
@@ -30,6 +33,7 @@ public class CurrentBranchHighlighter implements VcsLogHighlighter {
@NotNull private final VcsLogUiProperties myUiProperties;
@NotNull private final VcsLogDataHolder myDataHolder;
@NotNull private final VcsLogFilterUi myFilterUi;
@Nullable private String mySingleFilteredBranch;
public CurrentBranchHighlighter(@NotNull VcsLogDataHolder logDataHolder,
@NotNull VcsLogUiProperties uiProperties,
@@ -37,6 +41,19 @@ public class CurrentBranchHighlighter implements VcsLogHighlighter {
myDataHolder = logDataHolder;
myUiProperties = uiProperties;
myFilterUi = filterUi;
// this code will look much simpler when history* branch is merged
((VcsLogClassicFilterUi)filterUi).getLogUi().addLogListener(new VcsLogListener() {
@Override
public void onChange(@NotNull VcsLogDataPack dataPack, boolean refreshHappened) {
VcsLogBranchFilter branchFilter = myFilterUi.getFilters().getBranchFilter();
mySingleFilteredBranch = branchFilter == null
? null
: VcsLogUtil
.getSingleFilteredBranch(branchFilter, dataPack.getRefs(), dataPack.getLogProviders().keySet());
}
});
}
@NotNull
@@ -48,7 +65,7 @@ public class CurrentBranchHighlighter implements VcsLogHighlighter {
VcsLogProvider provider = myDataHolder.getLogProvider(details.getRoot());
String currentBranch = provider.getCurrentBranch(details.getRoot());
VcsLogBranchFilter branchFilter = myFilterUi.getFilters().getBranchFilter();
if (currentBranch != null && (branchFilter == null || !isFilteredByCurrentBranch(currentBranch, branchFilter))) {
if (currentBranch != null && (branchFilter == null || !(currentBranch.equals(mySingleFilteredBranch)))) {
Condition<Hash> condition =
myDataHolder.getContainingBranchesGetter().getContainedInBranchCondition(currentBranch, details.getRoot());
if (condition.value(details.getId())) {
@@ -59,10 +76,6 @@ public class CurrentBranchHighlighter implements VcsLogHighlighter {
return VcsCommitStyle.DEFAULT;
}
private boolean isFilteredByCurrentBranch(@NotNull String currentBranch, @NotNull VcsLogBranchFilter branchFilter) {
return currentBranch.equals(branchFilter.getSingleFilteredBranch());
}
public static class Factory implements VcsLogHighlighterFactory {
@NotNull private static final String ID = "CURRENT_BRANCH";
@@ -25,17 +25,21 @@ import com.intellij.vcs.log.*;
import com.intellij.vcs.log.data.VcsLogBranchFilterImpl;
import com.intellij.vcs.log.data.VcsLogUiProperties;
import com.intellij.vcs.log.impl.VcsLogUtil;
import com.intellij.vcs.log.ui.VcsLogUiImpl;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.*;
public class BranchFilterPopupComponent extends MultipleValueFilterPopupComponent<VcsLogBranchFilter> {
@NotNull private final VcsLogUiImpl myUi;
private VcsLogClassicFilterUi.BranchFilterModel myBranchFilterModel;
public BranchFilterPopupComponent(@NotNull VcsLogUiProperties uiProperties,
public BranchFilterPopupComponent(@NotNull VcsLogUiImpl ui,
@NotNull VcsLogUiProperties uiProperties,
@NotNull VcsLogClassicFilterUi.BranchFilterModel filterModel) {
super("Branch", uiProperties, filterModel);
myUi = ui;
myBranchFilterModel = filterModel;
}
@@ -51,10 +55,16 @@ public class BranchFilterPopupComponent extends MultipleValueFilterPopupComponen
return tooltip(getTextValues(filter));
}
@Nullable
@NotNull
@Override
protected VcsLogBranchFilter createFilter(@NotNull Collection<String> values) {
return VcsLogBranchFilterImpl.fromTextPresentation(values);
return VcsLogBranchFilterImpl
.fromTextPresentation(values, ContainerUtil.map2Set(myUi.getDataPack().getRefs().getBranches(), new Function<VcsRef, String>() {
@Override
public String fun(VcsRef vcsRef) {
return vcsRef.getName();
}
}));
}
@Override
@@ -85,8 +95,10 @@ public class BranchFilterPopupComponent extends MultipleValueFilterPopupComponen
return actionGroup;
}
public static ActionGroup constructActionGroup(@NotNull VcsLogDataPack dataPack, @Nullable ActionGroup recentItemsGroup,
@NotNull Function<String, AnAction> actionGetter, @Nullable Collection<VirtualFile> visibleRoots) {
public static ActionGroup constructActionGroup(@NotNull VcsLogDataPack dataPack,
@Nullable ActionGroup recentItemsGroup,
@NotNull Function<String, AnAction> actionGetter,
@Nullable Collection<VirtualFile> visibleRoots) {
Groups groups = prepareGroups(dataPack, visibleRoots);
return getFilteredActionGroup(groups, recentItemsGroup, actionGetter);
}
@@ -107,7 +119,8 @@ public class BranchFilterPopupComponent extends MultipleValueFilterPopupComponen
return filteredGroups;
}
private static DefaultActionGroup getFilteredActionGroup(@NotNull Groups groups, @Nullable ActionGroup recentItems,
private static DefaultActionGroup getFilteredActionGroup(@NotNull Groups groups,
@Nullable ActionGroup recentItems,
@NotNull Function<String, AnAction> actionGetter) {
DefaultActionGroup actionGroup = new DefaultActionGroup();
for (String single : groups.singletonGroups) {
@@ -54,7 +54,7 @@ abstract class MultipleValueFilterPopupComponent<Filter extends VcsLogFilter> ex
@NotNull
protected abstract List<String> getAllValues();
@Nullable
@NotNull
protected abstract Filter createFilter(@NotNull Collection<String> values);
@NotNull
@@ -64,7 +64,9 @@ abstract class MultipleValueFilterPopupComponent<Filter extends VcsLogFilter> ex
if (!recentlyFilteredUsers.isEmpty()) {
group.addSeparator("Recent");
for (List<String> recentGroup : recentlyFilteredUsers) {
group.add(new PredefinedValueAction(recentGroup));
if (!recentGroup.isEmpty()) {
group.add(new PredefinedValueAction(recentGroup));
}
}
group.addSeparator();
}
@@ -124,7 +124,7 @@ public class VcsLogClassicFilterUi implements VcsLogFilterUi {
actionGroup.add(new FilterActionComponent(new Computable<JComponent>() {
@Override
public JComponent compute() {
return new BranchFilterPopupComponent(myUiProperties, myBranchFilterModel).initUi();
return new BranchFilterPopupComponent(myUi, myUiProperties, myBranchFilterModel).initUi();
}
}));
actionGroup.add(new FilterActionComponent(new Computable<JComponent>() {
@@ -205,6 +205,11 @@ public class VcsLogClassicFilterUi implements VcsLogFilterUi {
}
}
@NotNull
public VcsLogUi getLogUi() {
return myUi;
}
private static class TextFilterComponent extends DumbAwareAction implements CustomComponentAction {
private final TextFilterModel myFilterModel;
@@ -82,7 +82,7 @@ class VisiblePackBuilderTest {
3(4)
4()
}
val visiblePack = graph.build(filters(VcsLogBranchFilterImpl.fromTextPresentation(setOf("-master"))))
val visiblePack = graph.build(filters(VcsLogBranchFilterImpl.fromTextPresentation(setOf("-master"), setOf("master"))))
val visibleGraph = visiblePack.getVisibleGraph()
assertEquals(3, visibleGraph.getVisibleCommitCount())
assertDoesNotContain(visibleGraph, 1)
@@ -109,7 +109,7 @@ class VisiblePackBuilderTest {
}
graph.providers.entrySet().iterator().next().getValue().setFilteredCommitsProvider(func)
val visiblePack = graph.build(filters(VcsLogBranchFilterImpl.fromTextPresentation(setOf("-master")), userFilter(DEFAULT_USER)))
val visiblePack = graph.build(filters(VcsLogBranchFilterImpl.fromTextPresentation(setOf("-master"), setOf("master")), userFilter(DEFAULT_USER)))
val visibleGraph = visiblePack.getVisibleGraph()
assertEquals(3, visibleGraph.getVisibleCommitCount())
assertDoesNotContain(visibleGraph, 1)
@@ -181,7 +181,7 @@ class VisiblePackBuilderTest {
= VcsLogFilterCollectionImpl(branchFilter(branch), userFilter(user), null, null, null, null, null)
fun branchFilter(branch: List<String>?): VcsLogBranchFilterImpl? {
return if (branch != null) VcsLogBranchFilterImpl.fromTextPresentation(branch) else null
return if (branch != null) VcsLogBranchFilterImpl.fromTextPresentation(branch, branch.toHashSet()) else null
}
fun userFilter(user: VcsUser?): VcsLogUserFilter? {
@@ -29,6 +29,7 @@ import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.vcs.log.*;
import com.intellij.vcs.log.impl.HashImpl;
import com.intellij.vcs.log.impl.VcsLogUtil;
import com.intellij.vcs.log.ui.MergeCommitsHighlighter;
import git4idea.GitBranch;
import git4idea.commands.GitCommand;
@@ -89,7 +90,7 @@ public class DeepComparator implements Disposable {
}
else {
VcsLogBranchFilter branchFilter = myUi.getFilterUi().getFilters().getBranchFilter();
if (branchFilter == null || !myTask.myComparedBranch.equals(branchFilter.getSingleFilteredBranch())) {
if (branchFilter == null || !myTask.myComparedBranch.equals(VcsLogUtil.getSingleFilteredBranch(branchFilter, myUi.getDataPack().getRefs(), myUi.getDataPack().getLogProviders().keySet()))) {
stopAndUnhighlight();
}
}
@@ -64,7 +64,7 @@ public class DeepCompareAction extends ToggleAction implements DumbAware {
final DeepComparator dc = DeepComparator.getInstance(project, ui);
if (selected) {
VcsLogBranchFilter branchFilter = ui.getFilterUi().getFilters().getBranchFilter();
String singleBranchName = branchFilter != null ? branchFilter.getSingleFilteredBranch() : null;
String singleBranchName = branchFilter != null ? VcsLogUtil.getSingleFilteredBranch(branchFilter, ui.getDataPack().getRefs(), ui.getDataPack().getLogProviders().keySet()) : null;
if (singleBranchName == null) {
selectBranchAndPerformAction(ui.getDataPack(), e, new Consumer<String>() {
@Override
@@ -410,27 +410,14 @@ public class GitLogProvider implements VcsLogProvider {
GitRepository repository = getRepository(root);
assert repository != null : "repository is null for root " + root + " but was previously reported as 'ready'";
Collection<GitLocalBranch> localBranches = repository.getBranches().getLocalBranches();
Collection<String> localBranchNames = ContainerUtil.map(localBranches, new Function<GitLocalBranch, String>() {
@Override
public String fun(GitLocalBranch branch) {
return branch.getName();
}
});
Collection<GitRemoteBranch> remoteBranches = repository.getBranches().getRemoteBranches();
Collection<String> remoteBranchNames = ContainerUtil.map(remoteBranches, new Function<GitRemoteBranch, String>() {
@Override
public String fun(GitRemoteBranch branch) {
return branch.getNameForLocalOperations();
}
});
Collection<GitBranch> branches = ContainerUtil
.newArrayList(ContainerUtil.concat(repository.getBranches().getLocalBranches(), repository.getBranches().getRemoteBranches()));
Collection<String> branchNames = GitBranchUtil.convertBranchesToNames(branches);
Collection<String> predefinedNames = ContainerUtil.list("HEAD");
boolean atLeastOneBranchExists = false;
for (String branchName: ContainerUtil.concat(localBranchNames, remoteBranchNames, predefinedNames)) {
if (branchFilter.isShown(branchName)) {
for (String branchName : ContainerUtil.concat(branchNames, predefinedNames)) {
if (branchFilter.matches(branchName)) {
filterParameters.add(branchName);
atLeastOneBranchExists = true;
}
@@ -195,13 +195,13 @@ public class HgLogProvider implements VcsLogProvider {
boolean atLeastOneBranchExists = false;
for (String branchName : ContainerUtil.concat(branchNames, bookmarkNames, predefinedNames)) {
if (branchFilter.isShown(branchName)) {
if (branchFilter.matches(branchName)) {
filterParameters.add(HgHistoryUtil.prepareParameter("branch", branchName));
atLeastOneBranchExists = true;
}
}
if (branchFilter.isShown(HEAD_REFERENCE)) {
if (branchFilter.matches(HEAD_REFERENCE)) {
filterParameters.add(HgHistoryUtil.prepareParameter("branch", "."));
filterParameters.add("-r");
filterParameters.add("::."); //all ancestors for current revision;