Skip to content

Commit 6de5430

Browse files
mbienmbien
committed
Fix git file history log order
Git history entries are not necessarily sortable by commit date which the history tab used as column sort order. (rebase may change commit order, commits could in theory also have the same commit/author date etc) This adds a position field to the fetched history entries which is now used in the comparator instead of the date. Comparisons between local and git history items still use timestamps.
1 parent 71f18a1 commit 6de5430

File tree

6 files changed

+69
-47
lines changed

6 files changed

+69
-47
lines changed

‎ide/versioning.ui/src/org/netbeans/modules/versioning/ui/history/HistoryDiffView.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,7 @@ public synchronized boolean cancel() {
655655

656656
synchronized void schedule() {
657657
task = History.getInstance().getRequestProcessor().create(this);
658-
task.schedule(500);
658+
task.schedule(200);
659659
}
660660

661661
protected synchronized boolean isCancelled() {

‎ide/versioning.ui/src/org/netbeans/modules/versioning/ui/history/HistoryEntry.java‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,15 @@
3333
*/
3434
class HistoryEntry {
3535
private final VCSHistoryProvider.HistoryEntry entry;
36+
37+
// position in the log used for UI sorting. "Oldest" has position 0.
38+
private int pos;
3639
private final boolean local;
3740
private Map<VCSFileProxy, HistoryEntry> parents;
3841

39-
HistoryEntry(VCSHistoryProvider.HistoryEntry entry, boolean local) {
42+
HistoryEntry(VCSHistoryProvider.HistoryEntry entry, int pos, boolean local) {
4043
this.entry = entry;
44+
this.pos = pos;
4145
this.local = local;
4246
}
4347

@@ -52,6 +56,14 @@ public Object[] getLookupObjects() {
5256
return ret;
5357
}
5458
}
59+
60+
public int getPos() {
61+
return pos;
62+
}
63+
64+
public void setPos(int pos) {
65+
this.pos = pos;
66+
}
5567

5668
public String getUsernameShort() {
5769
return entry.getUsernameShort();
@@ -99,18 +111,13 @@ public boolean canEdit() {
99111
}
100112

101113
public synchronized HistoryEntry getParent(VCSFileProxy file) {
102-
HistoryEntry parent = null;
103-
if(parents == null) {
104-
parents = new HashMap<VCSFileProxy, HistoryEntry>();
105-
} else {
106-
parent = parents.get(file);
107-
}
108-
if(parent == null) {
109-
VCSHistoryProvider.HistoryEntry vcsParent = entry.getParentEntry(file);
110-
parent = vcsParent != null ? new HistoryEntry(vcsParent, local) : null;
111-
parents.put(file, parent);
114+
if (parents == null) {
115+
parents = new HashMap<>();
112116
}
113-
return parent;
117+
return parents.computeIfAbsent(file, k -> {
118+
VCSHistoryProvider.HistoryEntry vcsParent = entry.getParentEntry(k);
119+
return vcsParent != null ? new HistoryEntry(vcsParent, 0, local) : null;
120+
});
114121
}
115122

116123
public boolean isLocalHistory() {

‎ide/versioning.ui/src/org/netbeans/modules/versioning/ui/history/HistoryFileView.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -241,11 +241,13 @@ public void run() {
241241
if(vcsHistory == null || vcsHistory.length == 0) {
242242
return;
243243
}
244-
List<HistoryEntry> entries = new ArrayList<HistoryEntry>(vcsHistory.length);
244+
List<HistoryEntry> entries = new ArrayList<>(vcsHistory.length);
245+
246+
int pos = vcsHistory.length - 1;
245247
for (VCSHistoryProvider.HistoryEntry he : vcsHistory) {
246-
entries.add(new HistoryEntry(he, false));
248+
entries.add(new HistoryEntry(he, pos--, false));
247249
}
248-
rootNode.addVCSEntries(entries.toArray(new HistoryEntry[0]), 0);
250+
rootNode.addVCSEntries(entries.toArray(HistoryEntry[]::new), 0);
249251
} finally {
250252
rootNode.loadingVCSFinished(currentDateFrom);
251253
EventQueue.invokeLater(new Runnable() {
@@ -579,7 +581,7 @@ private HistoryEntry[] loadLHEntries(VCSFileProxy[] files) {
579581
VCSHistoryProvider.HistoryEntry[] vcsHistory = hp.getHistory(files, null);
580582
HistoryEntry[] history = new HistoryEntry[vcsHistory.length];
581583
for (int i = 0; i < vcsHistory.length; i++) {
582-
history[i] = new HistoryEntry(vcsHistory[i], true);
584+
history[i] = new HistoryEntry(vcsHistory[i], vcsHistory.length - 1 - i, true);
583585
}
584586
return history;
585587
}
@@ -808,11 +810,11 @@ public int compare(Node n1, Node n2) {
808810
} else if(HistoryRootNode.isLoadNext(n2) || HistoryRootNode.isWait(n2)) {
809811
return etc.isAscending() ? -1 : 1;
810812
}
811-
if(n1 instanceof Comparable) {
812-
return ((Comparable)n1).compareTo(n2);
813+
if (n1 instanceof Comparable comparable) {
814+
return comparable.compareTo(n2);
813815
}
814-
if(n2 instanceof Comparable) {
815-
return -((Comparable)n2).compareTo(n1);
816+
if (n2 instanceof Comparable comparable) {
817+
return -comparable.compareTo(n1);
816818
}
817819
return n1.getName().compareTo(n2.getName());
818820
}

‎ide/versioning.ui/src/org/netbeans/modules/versioning/ui/history/HistoryRootNode.java‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public class HistoryRootNode extends AbstractNode {
4545

4646
private static DateFormat dateFormat = DateFormat.getDateInstance();
4747

48-
private Map<Long, HistoryEntry> revisionEntries = new HashMap<Long, HistoryEntry>();
48+
private Map<String, HistoryEntry> revisionEntries = new HashMap<>();
4949

5050
private LoadNextNode loadNextNode;
5151
private WaitNode waitNode;
@@ -106,29 +106,37 @@ synchronized HistoryEntry getPreviousEntry(HistoryEntry entry) {
106106

107107
private void addEntries (HistoryEntry[] entries, boolean vcs, long removeThreshold) {
108108
// add new
109-
List<Node> nodes = new LinkedList<Node>();
109+
List<Node> nodes = new LinkedList<>();
110110
removeOldEntries(entries, !vcs, removeThreshold);
111111
for (HistoryEntry e : entries) {
112-
if(!revisionEntries.containsKey(e.getDateTime().getTime())) {
113-
revisionEntries.put(e.getDateTime().getTime(), e);
112+
String key = key(e);
113+
if (!revisionEntries.containsKey(key)) {
114+
revisionEntries.put(key, e);
114115
if(vcs) {
115116
vcsCount++;
116117
}
117118
nodes.add(RevisionNode.create(e));
118-
}
119+
} else {
120+
revisionEntries.get(key).setPos(e.getPos());
121+
}
119122
}
120123
if(loadNextNode != null) {
121124
loadNextNode.refreshMessage();
122125
}
123-
getChildren().add(nodes.toArray(new Node[0]));
126+
getChildren().add(nodes.toArray(Node[]::new));
127+
}
128+
129+
// note: uses rev+time since local history entries return "Local" as revision
130+
private String key(HistoryEntry entry) {
131+
return entry.getRevision() + "-" + entry.getDateTime().getTime();
124132
}
125133

126134
private void removeOldEntries (HistoryEntry[] entries, boolean isLocal, long removeThreshold) {
127-
Set<Long> timestamps = new HashSet<Long>(entries.length);
135+
Set<Long> timestamps = new HashSet<>(entries.length);
128136
for (HistoryEntry e : entries) {
129137
timestamps.add(e.getDateTime().getTime());
130138
}
131-
List<Node> toRemove = new ArrayList<Node>();
139+
List<Node> toRemove = new ArrayList<>();
132140
for (Node n : getChildren().getNodes()) {
133141
HistoryEntry e = n.getLookup().lookup(HistoryEntry.class);
134142
if (e != null && isLocal == e.isLocalHistory()) {
@@ -139,14 +147,14 @@ private void removeOldEntries (HistoryEntry[] entries, boolean isLocal, long rem
139147
History.LOG.log(Level.FINE, "Removing obsolete history entry: {0} {1}", //NOI18N
140148
new Object[] { e.getRevisionShort(), e.getMessage() });
141149
toRemove.add(n);
142-
revisionEntries.remove(time);
150+
revisionEntries.remove(key(e));
143151
if (!isLocal) {
144152
vcsCount--;
145153
}
146154
}
147155
}
148156
}
149-
getChildren().remove(toRemove.toArray(new Node[0]));
157+
getChildren().remove(toRemove.toArray(Node[]::new));
150158
}
151159

152160
public synchronized void addWaitNode() {

‎ide/versioning.ui/src/org/netbeans/modules/versioning/ui/history/RevisionNode.java‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ class RevisionNode extends AbstractNode implements Comparable {
4747
static final String PROPERTY_NAME_USER = "user"; // NOI18N
4848
static final String PROPERTY_NAME_VERSION = "version"; // NOI18N
4949

50-
private HistoryEntry entry;
51-
private static DateFormat dateFormat = DateFormat.getDateTimeInstance();
52-
private static DateFormat timeFormat = DateFormat.getTimeInstance();
50+
private final HistoryEntry entry;
51+
private static final DateFormat dateFormat = DateFormat.getDateTimeInstance();
52+
private static final DateFormat timeFormat = DateFormat.getTimeInstance();
5353

5454
private RevisionNode(HistoryEntry entry, Lookup l) {
5555
super(createChildren(entry), l);
@@ -58,7 +58,7 @@ private RevisionNode(HistoryEntry entry, Lookup l) {
5858
}
5959

6060
static RevisionNode create(HistoryEntry entry) {
61-
List<Object> lookup = new LinkedList<Object>();
61+
List<Object> lookup = new LinkedList<>();
6262
VCSFileProxy[] proxies = entry.getFiles();
6363
for (VCSFileProxy proxy : proxies) {
6464
lookup.add(proxy);
@@ -69,7 +69,7 @@ static RevisionNode create(HistoryEntry entry) {
6969
}
7070
lookup.addAll(Arrays.asList(entry.getLookupObjects()));
7171
lookup.add(entry);
72-
return new RevisionNode(entry, Lookups.fixed(lookup.toArray(new Object[0])));
72+
return new RevisionNode(entry, Lookups.fixed(lookup.toArray(Object[]::new)));
7373
}
7474

7575
private static Children createChildren(HistoryEntry entry) {
@@ -116,11 +116,11 @@ public String toString() {
116116

117117
static String getFormatedDate(HistoryEntry se) {
118118
int day = getDay(se.getDateTime().getTime());
119-
switch(day) {
120-
case 0: return NbBundle.getMessage(RevisionNode.class, "LBL_Today", new Object[] {timeFormat.format(se.getDateTime())});
121-
case 1: return NbBundle.getMessage(RevisionNode.class, "LBL_Yesterday", new Object[] {timeFormat.format(se.getDateTime())});
122-
default: return dateFormat.format(se.getDateTime());
123-
}
119+
return switch (day) {
120+
case 0 -> NbBundle.getMessage(RevisionNode.class, "LBL_Today", new Object[] {timeFormat.format(se.getDateTime())});
121+
case 1 -> NbBundle.getMessage(RevisionNode.class, "LBL_Yesterday", new Object[] {timeFormat.format(se.getDateTime())});
122+
default -> dateFormat.format(se.getDateTime());
123+
};
124124
}
125125

126126
private static int getDay(long ts) {
@@ -154,11 +154,16 @@ public int compareTo(Object obj) {
154154
return -1;
155155
}
156156
RevisionNode node = (RevisionNode) obj;
157-
return entry.getDateTime().compareTo(node.entry.getDateTime());
157+
158+
if (entry.isLocalHistory() || node.entry.isLocalHistory()) {
159+
return entry.getDateTime().compareTo(node.entry.getDateTime());
160+
} else {
161+
return Integer.compare(entry.getPos(), node.entry.getPos());
162+
}
158163
}
159164

160165
class MessageProperty extends PropertySupport.ReadOnly<TableEntry> {
161-
private TableEntry te;
166+
private final TableEntry te;
162167
public MessageProperty() {
163168
super(PROPERTY_NAME_LABEL, TableEntry.class, NbBundle.getMessage(RevisionNode.class, "LBL_LabelProperty_Name"), NbBundle.getMessage(RevisionNode.class, "LBL_LabelProperty_Desc"));
164169
te = new TableEntry() {
@@ -275,7 +280,7 @@ public boolean equals(Object obj) {
275280
}
276281

277282
class UserProperty extends PropertySupport.ReadOnly<TableEntry> {
278-
private TableEntry te;
283+
private final TableEntry te;
279284
public UserProperty() {
280285
super(PROPERTY_NAME_USER, TableEntry.class, NbBundle.getMessage(RevisionNode.class, "LBL_UserProperty_Name"), NbBundle.getMessage(RevisionNode.class, "LBL_UserProperty_Desc"));
281286
te = new UserEntry(entry);
@@ -413,15 +418,15 @@ static class FileNode extends AbstractNode implements Comparable {
413418
}
414419

415420
private static Lookup createLookup(VCSFileProxy proxy, HistoryEntry entry) {
416-
List<Object> lookup = new LinkedList<Object>();
421+
List<Object> lookup = new LinkedList<>();
417422
lookup.add(proxy);
418423
File f = proxy.toFile();
419424
if(f != null) {
420425
lookup.add(f);
421426
}
422427
lookup.add(entry);
423428
lookup.addAll(Arrays.asList(entry.getLookupObjects()));
424-
return Lookups.fixed(lookup.toArray(new Object[0]));
429+
return Lookups.fixed(lookup.toArray(Object[]::new));
425430
}
426431

427432
@Override

‎ide/versioning.ui/test/unit/src/org/netbeans/modules/versioning/ui/history/HistoryTestKit.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@
2626
*/
2727
public class HistoryTestKit {
2828
public static Node createHistoryNode(org.netbeans.modules.versioning.core.spi.VCSHistoryProvider.HistoryEntry entry) {
29-
return RevisionNode.create(new HistoryEntry(entry, false));
29+
return RevisionNode.create(new HistoryEntry(entry, 42, false));
3030
}
3131
}

0 commit comments

Comments
 (0)