Skip to content

Commit fb1c15c

Browse files
committed
fixing little bugs in ModeVersions, updating notes
1 parent 46d8815 commit fb1c15c

File tree

6 files changed

+149
-142
lines changed

6 files changed

+149
-142
lines changed

src/org/argmap/client/ArgMap.java

Lines changed: 38 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -21,31 +21,17 @@
2121
import com.google.gwt.user.client.ui.TabLayoutPanel;
2222
import com.google.gwt.user.client.ui.Widget;
2323

24+
//TODO: do some basic testing of versioning of deleted top level nodes
25+
/* TODO transition ModeEdit to the new dummy node approach and get rid of ViewDummyVer */
26+
2427
/* TODO: ModeVersions: for some reason child nodes of a deleted and re-added link are not showing up in the first attachement of the node
2528
* so you have to browse to a later point in time, and open up the link, and the go back in time, in order to open up the link node.
2629
* (Not sure if the problem is confined to links or not.)
2730
*/
28-
/*TODO: what happens in ModeVersions if I first link to a node as negated, then delete it, then link to it as non-negated and delete that
29-
* (or vice-versa)? Nodes are stores by ID in the deletedNodes list. Since its stored in a hashmap, storing the same deleted node twice will
30-
* result in only one copy. Will that work for storing the history? One thing I want to work out below is only storing the history for a
31-
* link that is relevant to the time that it exists in the tree so there aren't a lot of extra changes in the change list. Will having
32-
* only one copy complicate that? If so maybe I could save in a list or a hashmap keyed on date instead of ID (or ID and date, or something)...
33-
*
34-
* Regardless, as far as negation is concerned, if there is only one copy, I can't depend on it to have the right negated value when
35-
* it is re-added to the tree for the first time.
36-
*/
37-
/*TODO: test opening negated links in versions mode (probably won't work!!!); also make sure to
38-
* test adding a link as negated, removing it, adding at not negated, (and the reverse order) and see if
39-
* versions mode gets it right.
40-
*/
41-
4231
/*TODO: client throws an exception when removing a link and re-adding it (at least if you make a single change to the link in the interim
4332
* I haven't tested other scenarios yet) */
44-
/* TODO transition ModeEdit to the new dummy node approach and get rid of ViewDummyVer */
45-
/* TODO when a root proposition is first added, program suggest using it instead (as a link) of itself! */
4633
//TODO: fix versions mode formating of links
47-
//TODO: versioning root node with no modifications/adds throws exception because of empty change list
48-
//TODO: do some basic testing of versioning of deleted top level nodes
34+
4935
/*TODO: fix exceptions when opening circular links in versions mode and continue testings version mode's handling of circular linking*/
5036
/*TODO: track down exceptions in ModeVersion (unrelated to circular linking)*/
5137
/*TODO: versions mode: update link coloring depending on whether a proposition is currently a link?
@@ -58,7 +44,25 @@
5844
* proposition it would update the props link status. If it was an argument it would just do what it currently does.
5945
* This way the system would work for propositions whose link coloring is changed by a linking/unlinking not within
6046
* the tree being viewed by the user...
47+
*
48+
* This seems to be related too: TO DO: ModeVersions: undoing unlinks does not restore the link's yellow color if the link *currently* is not
49+
* linked to by more than one argument because the server sends the current proposition which
50+
* indicates a link count of 1. This could be addressed by having a proposition's link/unlink
51+
* events also be owned by that proposition in VersionsMode, and hiding them probably (because
52+
* they may be interesting only in regards to the color of the node because the unlink/link
53+
* events may be with regards to arguments that are not currently opened) and treating
54+
* them as only coloring events when moving forwards and backwards (thus in some cases there
55+
* would be two link/unlink ViewChanges, one for the proposition whose link count is being updated
56+
* and one for the argument which needs to either display or hide the linked proposition). The forwards/backwards
57+
* methods could distinguish between links/unlinks that were to be treated only as coloring events versus
58+
* ones that would be treated as displaying/removing the linked proposition based on whether the ViewNodeVer
59+
* object contained in the ViewChange object was a ViewPropVer or a ViewArgVer. If it's a ViewArgVer
60+
* it would display/remove the linked proposition. If it's a ViewPropVer it would simply update the link
61+
* count and change the color from yellow to white if it falls below 2, and vice versa. This would require a
62+
* change on the server to return the coloring events for a proposition, and would require changes on the client
63+
* to insert the coloring events into the timeMachineMap and so forth.
6164
*/
65+
/* TODO when a root proposition is first added, program suggest using it instead (as a link) of itself! */
6266
//TODO: comment the hell out of versions mode!!!!!!! someday I'll need to change it...
6367

6468
//TODO: add 'real' example arguments for demonstration (for instance my argument about legalizing unauthorized access)
@@ -82,23 +86,6 @@
8286
//TODO: try running speed tracer
8387
//TODO: make batch open icon visible, and open tree a few layers deep regardless of whether mouse over node is already loaded
8488
//TODO: batch open icon not visible/clickable on props that reach right screen edge
85-
/*TODO: ModeVersions: undoing unlinks does not restore the link's yellow color if the link *currently* is not
86-
* linked to by more than one argument because the server sends the current proposition which
87-
* indicates a link count of 1. This could be addressed by having a proposition's link/unlink
88-
* events also be owned by that proposition in VersionsMode, and hiding them probably (because
89-
* they may be interesting only in regards to the color of the node because the unlink/link
90-
* events may be with regards to arguments that are not currently opened) and treating
91-
* them as only coloring events when moving forwards and backwards (thus in some cases there
92-
* would be two link/unlink ViewChanges, one for the proposition whose link count is being updated
93-
* and one for the argument which needs to either display or hide the linked proposition). The forwards/backwards
94-
* methods could distinguish between links/unlinks that were to be treated only as coloring events versus
95-
* ones that would be treated as displaying/removing the linked proposition based on whether the ViewNodeVer
96-
* object contained in the ViewChange object was a ViewPropVer or a ViewArgVer. If it's a ViewArgVer
97-
* it would display/remove the linked proposition. If it's a ViewPropVer it would simply update the link
98-
* count and change the color from yellow to white if it falls below 2, and vice versa. This would require a
99-
* change on the server to return the coloring events for a proposition, and would require changes on the client
100-
* to insert the coloring events into the timeMachineMap and so forth.
101-
*/
10289
//TODO: figure out how to make server log more than info, warn and severe while in hosted mode...
10390

10491
/*TODO: move changes from propID/argID to parentID/childID (this will make querying more efficient: want
@@ -148,23 +135,23 @@
148135
public class ArgMap implements EntryPoint, UncaughtExceptionHandler,
149136
SelectionHandler<Integer> {
150137

151-
private ModeEdit editMode;
138+
private ModeEdit modeEdit;
152139

153140
private DockLayoutPanel mainPanel;
154141
private TabLayoutPanel modePanel;
155142
private HorizontalPanel loginPanel;
156143

157144
private HTML messageArea;
158145
private MultiMap<String, Message> messageMap;
159-
private ModeVersions versionsMode;
146+
private ModeVersions modeVersions;
160147
private static ArgMap argMap;
161148
private boolean loggedIn;
162149

163150
public void onModuleLoad() {
164151
argMap = this;
165152
messageArea = new HTML();
166153
messageMap = new MultiMap<String, Message>();
167-
editMode = new ModeEdit(this);
154+
modeEdit = new ModeEdit(this);
168155

169156
GWT.runAsync(new AsyncRunCallback() {
170157

@@ -175,7 +162,7 @@ public void onSuccess() {
175162
loginPanel = new HorizontalPanel();
176163

177164
GWT.setUncaughtExceptionHandler(ArgMap.this);
178-
modePanel.add(editMode, "Find And Collaborate");
165+
modePanel.add(modeEdit, "Find And Collaborate");
179166

180167
modePanel.addSelectionHandler(ArgMap.this);
181168

@@ -258,29 +245,33 @@ public static boolean loggedIn() {
258245
return argMap.loggedIn;
259246
}
260247

248+
public void gotoModeEdit() {
249+
modePanel.selectTab(modeEdit);
250+
}
251+
261252
public void showVersions() {
262253
if (!versionsIsDisplayed()) {
263254
GWT.runAsync(new AsyncRunCallback() {
264255

265256
@Override
266257
public void onSuccess() {
267-
if (versionsMode == null) {
268-
versionsMode = new ModeVersions(editMode);
258+
if (modeVersions == null) {
259+
modeVersions = new ModeVersions(ArgMap.this);
269260
}
270-
modePanel.insert(versionsMode, "History", 1);
261+
modePanel.insert(modeVersions, "History", 1);
271262
}
272263
});
273264
}
274265
}
275266

276267
public void hideVersions() {
277268
if (versionsIsDisplayed()) {
278-
modePanel.remove(versionsMode);
269+
modePanel.remove(modeVersions);
279270
}
280271
}
281272

282273
private boolean versionsIsDisplayed() {
283-
if (modePanel.getWidgetIndex(versionsMode) == -1) {
274+
if (modePanel.getWidgetIndex(modeVersions) == -1) {
284275
return false;
285276
} else {
286277
return true;
@@ -410,14 +401,14 @@ public void onUncaughtException(Throwable e) {
410401
}
411402

412403
public ModeEdit getModeEdit() {
413-
return editMode;
404+
return modeEdit;
414405
}
415406

416407
@Override
417408
public void onSelection(SelectionEvent<Integer> event) {
418409
Widget widget = modePanel.getWidget(modePanel.getSelectedIndex());
419-
if (widget == versionsMode) {
420-
versionsMode.displayVersions();
410+
if (widget == modeVersions) {
411+
modeVersions.displayVersions();
421412
}
422413
}
423414

src/org/argmap/client/MessageDialog.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,9 @@ public void onClick(ClickEvent event) {
4444
});
4545
dialogContents.add(closeButton);
4646
}
47+
48+
public void centerAndShow() {
49+
center();
50+
show();
51+
}
4752
}

src/org/argmap/client/ModeVersions.java

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public class ModeVersions extends ResizeComposite implements
154154
*/
155155

156156
private final ListBox versionList = new ListBox();
157-
private final ModeEdit editMode;
157+
private final ArgMap argMap;
158158
private ArgTree treeClone = null;
159159
private final ScrollPanel treePanel = new ScrollPanel();
160160
private final int LIST_WIDTH = 20;
@@ -207,13 +207,13 @@ public class ModeVersions extends ResizeComposite implements
207207
*/
208208
private static Date now = new Date();
209209

210-
public ModeVersions(ModeEdit editModePair) {
210+
public ModeVersions(ArgMap argMap) {
211211
super();
212212

213213
/************************************
214214
* setup the history browsing panel *
215215
************************************/
216-
this.editMode = editModePair;
216+
this.argMap = argMap;
217217
DockLayoutPanel historyPanel = new DockLayoutPanel(Unit.EM);
218218

219219
historyPanel.addWest(versionList, LIST_WIDTH);
@@ -274,7 +274,7 @@ public void displayVersions() {
274274
}
275275
List<Proposition> props = new ArrayList<Proposition>();
276276
List<Argument> args = new ArrayList<Argument>();
277-
editMode.getOpenPropsAndArgs(props, args);
277+
argMap.getModeEdit().getOpenPropsAndArgs(props, args);
278278
ServerComm.getChanges(props, args,
279279
new ServerComm.LocalCallback<NodeChangesMaps>() {
280280

@@ -296,13 +296,18 @@ public void call(NodeChangesMaps changesMaps) {
296296
* the call back just keep calling something like wait()
297297
* until it finds that the clone is finished?
298298
*/
299-
editMode.buildTreeCloneOfOpenNodes(treeClone);
299+
argMap.getModeEdit().buildTreeCloneOfOpenNodes(
300+
treeClone);
300301

301302
treePanel.add(treeClone);
302303

303304
timeMachineMap = prepTreeWithDeletedNodesAndChangesAndBuildTimeMachineMap(
304305
treeClone, changesMaps);
305306

307+
if (ifTimeMachineMapIsEmptyShowMessageAndReturnToModeEdit()) {
308+
return;
309+
}
310+
306311
if (Log.on) {
307312
Log treeLog = Log.getLog("vm.dv.tree");
308313
treeClone.logTree(log);
@@ -325,7 +330,7 @@ public void call(NodeChangesMaps changesMaps) {
325330
* TODO what is this onChange() called for? get rid of
326331
* it?
327332
*/
328-
onChange(null);
333+
// onChange(null);
329334
treeClone.resetState();
330335
logTreeWithChanges();
331336
log.finish();
@@ -334,6 +339,20 @@ public void call(NodeChangesMaps changesMaps) {
334339

335340
}
336341

342+
private boolean ifTimeMachineMapIsEmptyShowMessageAndReturnToModeEdit() {
343+
if (timeMachineMap.isEmpty()) {
344+
argMap.gotoModeEdit();
345+
new MessageDialog(
346+
"Selection Has No History",
347+
"The server reports that the nodes you have selected have no change history, because no one has "
348+
+ "ever edited them. History viewing tab has exited.")
349+
.centerAndShow();
350+
return true;
351+
} else {
352+
return false;
353+
}
354+
}
355+
337356
/*
338357
* this method takes care of down-loading the changes from the server and
339358
* setting up a clone of the edit tree to work with.
@@ -378,6 +397,10 @@ public void call(NodeChangesMapsAndRootChanges changes) {
378397
timeMachineMap = prepTreeWithDeletedNodesAndChangesAndBuildTimeMachineMap(
379398
treeClone, changes.nodeChangesMaps);
380399

400+
if (ifTimeMachineMapIsEmptyShowMessageAndReturnToModeEdit()) {
401+
return;
402+
}
403+
381404
mapPropContent = new HashMap<ViewChange, String>();
382405
mapArgTitle = new HashMap<ViewChange, String>();
383406
mapArgIndex = new HashMap<ViewChange, Integer>();
@@ -1023,8 +1046,11 @@ public void mergeLoadedNodes(ViewNodeVer viewNodeVer,
10231046
(ViewNodeVer) viewNodeVer.getOldestAncestor(),
10241047
new TimePeriods());
10251048

1026-
zoomToCurrentChangeAndReloadChangeList(viewNodeVer, viewChanges,
1027-
viewChanges.lastKey());
1049+
if (!viewChanges.isEmpty()) {
1050+
zoomToCurrentChangeAndMergeChangeMaps(viewNodeVer, viewChanges, viewChanges.lastKey());
1051+
}
1052+
1053+
reloadChangeListAfterOpen(viewNodeVer);
10281054
// zoomToCurrentChangeAndReloadChangeList(viewNodeVer, viewChanges,
10291055
// Long.MAX_VALUE);
10301056
}
@@ -1127,23 +1153,23 @@ public void call(Map<Long, NodeWithChanges> nodesWithChanges) {
11271153
SortedMultiMap<Long, ViewChange> subTreeChanges = new SortedMultiMap<Long, ViewChange>();
11281154
recursiveGetViewChanges(viewNodeVer, subTreeChanges, true, log);
11291155

1130-
zoomToCurrentChangeAndReloadChangeList(viewNodeVer, subTreeChanges,
1156+
zoomToCurrentChangeAndMergeChangeMaps(viewNodeVer, subTreeChanges,
11311157
viewNodeVer.getChangeIDOnClose());
1158+
reloadChangeListAfterOpen(viewNodeVer);
11321159

11331160
}
11341161
log.finish();
11351162
logTreeWithChanges();
11361163
}
11371164

1138-
public void zoomToCurrentChangeAndReloadChangeList(ViewNodeVer viewNodeVer,
1165+
public void zoomToCurrentChangeAndMergeChangeMaps(ViewNodeVer viewNodeVer,
11391166
SortedMultiMap<Long, ViewChange> subTreeChanges, Long startChangeID) {
1140-
1141-
viewNodeVer.setOpen(true);
1142-
11431167
travelFromChangeToChange(startChangeID, currentChangeID, subTreeChanges);
1144-
11451168
timeMachineMap.putAll(subTreeChanges);
1169+
}
11461170

1171+
public void reloadChangeListAfterOpen(ViewNodeVer viewNodeVer) {
1172+
viewNodeVer.setOpen(true);
11471173
for (ViewChange viewChange : viewNodeVer.getViewChangeHideList()) {
11481174
viewChange.hidden = false;
11491175
}
@@ -1254,7 +1280,8 @@ public void onChange(ChangeEvent event) {
12541280
public void travelFromChangeToChange(Long currentChangeID,
12551281
Long newChangeID, SortedMultiMap<Long, ViewChange> changes) {
12561282
Log log = Log.getLog("tm.ttd");
1257-
if (newChangeID < currentChangeID) {
1283+
int comparison = newChangeID.compareTo(currentChangeID);
1284+
if (comparison < 0) {
12581285
log.log("traveling back to Change.id:" + newChangeID
12591286
+ "; from Change.id:" + currentChangeID);
12601287
/*
@@ -1283,7 +1310,7 @@ public void travelFromChangeToChange(Long currentChangeID,
12831310
newChangeID, false, currentChangeID, true);
12841311
Collections.reverse(reverseList);
12851312
moveTreeBackwards(reverseList, log);
1286-
} else if (newChangeID > currentChangeID) {
1313+
} else if (comparison > 0) {
12871314
/*
12881315
* the current tree shows the tree after the change highlighted was
12891316
* made. currentDate corresponds to the change higlighted. To move

0 commit comments

Comments
 (0)