Skip to content

Commit

Permalink
[INTERNAL][E] Avoid annotation split handling
Browse files Browse the repository at this point in the history
Instead of splitting big annotation if we insert new annotation,
this handling only uses annotations of size 1 and stores multiple
annotations as one entry in the history.
Therefore, the split handling is removed and the issue with
auto-expanding annotations is fixed.
Fix: #32

Furthermore, the history handling for split view is fixed.
The old handling assumed that the entries that are removed
from the history and have to be removed from the models use
the same model as specified in the parameter of insertAnnotation.
The new handling uses the model assigned to the annotation to
remove the annotation.
  • Loading branch information
m273d15 committed Mar 25, 2020
1 parent 5f34ee4 commit f411568
Show file tree
Hide file tree
Showing 5 changed files with 303 additions and 244 deletions.
3 changes: 0 additions & 3 deletions eclipse/src/saros/editor/EditorManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -1074,9 +1074,6 @@ private void replaceText(SPath path, int offset, String replacedText, String tex
contributionAnnotationManager.insertAnnotation(model, offset, text.length(), source);
}
}

IAnnotationModel model = provider.getAnnotationModel(input);
contributionAnnotationManager.insertAnnotation(model, offset, text.length(), source);
} finally {
provider.disconnect(input);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.ListIterator;
import java.util.Queue;
Expand All @@ -12,70 +11,59 @@
import org.eclipse.jface.text.source.IAnnotationModel;
import saros.editor.annotations.ContributionAnnotation;

/** */
/** A history for the contribution annotations that has a fixed-size and uses a FIFO approach. */
class ContributionAnnotationHistory {

private static final Logger log = Logger.getLogger(ContributionAnnotationHistory.class);
Queue<List<ContributionAnnotation>> queue;
private Queue<List<ContributionAnnotation>> queue;
private int maxHistoryLength;

protected ContributionAnnotationHistory(int maxHistoryLength) {
ContributionAnnotationHistory(final int maxHistoryLength) {
this.queue = new ArrayDeque<>(maxHistoryLength);
this.maxHistoryLength = maxHistoryLength;
}

/** @return current size of the history */
int getSize() {
return queue.size();
}

/**
* Removes all entries in the history and returns an aggregated list of all contribution
* annotations in the history.
* annotations in the history
*
* @return A flattened all history entries
* @return a flattened list of all history entries
*/
protected List<ContributionAnnotation> clear() {
List<ContributionAnnotation> content = new ArrayList<>();
List<ContributionAnnotation> clear() {
final List<ContributionAnnotation> content = new ArrayList<>();
while (!queue.isEmpty()) {
content.addAll(queue.remove());
}
return content;
}

/** If the history */
protected Pair<IAnnotationModel, List<ContributionAnnotation>> removeHistoryEntry() {

Pair<IAnnotationModel, List<ContributionAnnotation>> removedEntry = null;

if (queue.size() == maxHistoryLength) {
List<ContributionAnnotation> annotations = queue.remove();
IAnnotationModel model = annotations.get(0).getModel();
for (ContributionAnnotation a : annotations) {
if (a.getModel() != model) throw new RuntimeException("Different Models");
}
removedEntry = new ImmutablePair<>(model, annotations);
}

return removedEntry;
}

/*
* Replaces the an annotation in the history with another one.
* E.g. if we want to replace the annotation D with D':
* [[A, B], [C], [D, E], [F], [G]] -> [[A, B], [C], [D', E], [F], [G]]
/**
* Replaces an annotation in the history with another one. E.g. if we want to replace the
* annotation D with D': [[A, B], [C], [D, E], [F], [G]] -> [[A, B], [C], [D', E], [F], [G]]
*
* @param oldAnnotation the annotation that has to be replaced.
* @param newAnnotation the annotation that has to be inserted.
*/
protected void replaceInHistory(
void replaceInHistory(
final ContributionAnnotation oldAnnotation, final ContributionAnnotation newAnnotation) {

for (List<ContributionAnnotation> entry : queue) {
for (final List<ContributionAnnotation> entry : queue) {

for (final ListIterator<ContributionAnnotation> annotationsLit = entry.listIterator();
annotationsLit.hasNext(); ) {
final ContributionAnnotation annotation = annotationsLit.next();
for (final ListIterator<ContributionAnnotation> annotationsListIt = entry.listIterator();
annotationsListIt.hasNext(); ) {
final ContributionAnnotation annotation = annotationsListIt.next();

if (annotation.equals(oldAnnotation)) {
annotationsLit.remove();
annotationsListIt.remove();

assert oldAnnotation.getSource().equals(newAnnotation.getSource());

annotationsLit.add(newAnnotation);
annotationsListIt.add(newAnnotation);
return;
}
}
Expand All @@ -88,16 +76,30 @@ protected void replaceInHistory(
+ oldAnnotation.getSource());
}

protected void addNewEntry(ContributionAnnotation annotation) {
addNewEntry(Arrays.asList(annotation));
/**
* @param annotations annotations to add as one entry into the history
* @return If an entry has to be removed from the history in order to add the new one, the removed
* entry is returned with the corresponding {@link IAnnotationModel}. Otherwise return <code>
* null</code>.
*/
Pair<IAnnotationModel, List<ContributionAnnotation>> addNewEntry(
final List<ContributionAnnotation> annotations) {

Pair<IAnnotationModel, List<ContributionAnnotation>> removedEntry = removeEntryIfFull();
queue.add(annotations);
return removedEntry;
}

protected void addNewEntry(List<ContributionAnnotation> annotations) {
if (queue.size() >= maxHistoryLength) {
throw new IllegalStateException(
"The queue already contains the " + "allowed number of annotations.");
private Pair<IAnnotationModel, List<ContributionAnnotation>> removeEntryIfFull() {

Pair<IAnnotationModel, List<ContributionAnnotation>> removedEntry = null;

if (queue.size() == maxHistoryLength) {
final List<ContributionAnnotation> annotations = queue.remove();
final IAnnotationModel model = annotations.get(0).getModel();
removedEntry = new ImmutablePair<>(model, annotations);
}

queue.add(annotations);
return removedEntry;
}
}
137 changes: 83 additions & 54 deletions eclipse/src/saros/editor/internal/ContributionAnnotationManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.commons.lang3.tuple.ImmutablePair;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.log4j.Logger;
import org.eclipse.jface.preference.IPreferenceStore;
Expand Down Expand Up @@ -55,7 +56,7 @@ public class ContributionAnnotationManager {
private final ISessionListener sessionListener =
new ISessionListener() {
@Override
public void userLeft(User user) {
public void userLeft(final User user) {
removeAnnotationsForUser(user);
}
};
Expand Down Expand Up @@ -84,7 +85,7 @@ public void run() {
};

public ContributionAnnotationManager(
ISarosSession sarosSession, IPreferenceStore preferenceStore) {
final ISarosSession sarosSession, final IPreferenceStore preferenceStore) {

this.sarosSession = sarosSession;
this.preferenceStore = preferenceStore;
Expand All @@ -96,53 +97,108 @@ public ContributionAnnotationManager(
}

/**
* Inserts a contribution annotation to given model if there is not already a contribution
* Inserts contribution annotations to given model if there is not already a contribution
* annotation at given position. This method should be called after the text has changed.
*
* @param model to add the annotation to.
* @param offset start of the annotation to add.
* @param length length of the annotation.
* @param source of the annotation.
*/
public void insertAnnotation(IAnnotationModel model, int offset, int length, User source) {
public void insertAnnotation(
final IAnnotationModel model, final int offset, final int length, final User source) {

if (!contribtionAnnotationsEnabled) return;
// Return if length == 0, (called after a deletion was performed)
if (!contribtionAnnotationsEnabled || length <= 0) return;

if (length < 0) // why is 0 len allowed ?
return;
final ContributionAnnotationHistory history = getHistory(source);

/* Return early if there already is an annotation at that offset */
for (Iterator<Annotation> it = model.getAnnotationIterator(); it.hasNext(); ) {
Annotation annotation = it.next();
final Map<ContributionAnnotation, Position> annotationsToAdd =
createAnnotationsForContributionRange(model, offset, length, source);

if (annotation instanceof ContributionAnnotation
&& model.getPosition(annotation).includes(offset)
&& ((ContributionAnnotation) annotation).getSource().equals(source)) {
return;
final Pair<IAnnotationModel, List<ContributionAnnotation>> annotationsToRemove =
history.addNewEntry(new ArrayList<>(annotationsToAdd.keySet()));

// Case 1: An insert operation is performed and the history is full
if (annotationsToRemove != null) {
final IAnnotationModel annotationsToRemoveModel = annotationsToRemove.getLeft();

if (annotationsToRemoveModel.equals(model)) {
annotationModelHelper.replaceAnnotationsInModel(
annotationsToRemoveModel, annotationsToRemove.getRight(), annotationsToAdd);
} else {
annotationModelHelper.replaceAnnotationsInModel(
annotationsToRemoveModel, annotationsToRemove.getRight(), Collections.emptyMap());
annotationModelHelper.replaceAnnotationsInModel(
model, Collections.emptyList(), annotationsToAdd);
}
}
// Case 2: An insert operation is performed and the history is not full
else {
annotationModelHelper.replaceAnnotationsInModel(
model, Collections.emptyList(), annotationsToAdd);
}
}

/**
* Creates contribution annotations with length 1 for each char contained in the range defined by
* {@code offset} and {@code length}.
*
* @param model the model that is assigned to the created contribution annotations.
* @param offset start of the annotation range.
* @param length length of the annotation range.
* @param source of the annotation.
* @return a map containing the annotations and their positions
*/
private Map<ContributionAnnotation, Position> createAnnotationsForContributionRange(
final IAnnotationModel model, final int offset, final int length, final User source) {

final Map<ContributionAnnotation, Position> annotationsToAdd = new HashMap<>();

for (int i = 0; i < length; i++) {
final Pair<ContributionAnnotation, Position> positionedAnnotation =
createPositionedAnnotation(model, offset + i, source);
if (positionedAnnotation == null) continue;

annotationsToAdd.put(positionedAnnotation.getKey(), positionedAnnotation.getValue());
}

return annotationsToAdd;
}

/**
* Creates a contribution annotations with length 1 at position {@code offset}.
*
* @param model to add the annotation to.
* @param offset start of the annotation to add.
* @param source of the annotation.
* @return a pair containing the annotation and its position.
*/
private Pair<ContributionAnnotation, Position> createPositionedAnnotation(
final IAnnotationModel model, final int offset, final User source) {
final int ANNOTATION_LENGTH = 1;

addContributionAnnotation(
new ContributionAnnotation(source, model), new Position(offset, length));
return new ImmutablePair<ContributionAnnotation, Position>(
new ContributionAnnotation(source, model), new Position(offset, ANNOTATION_LENGTH));
}

/**
* Refreshes all contribution annotations in the model by removing and reinserting them.
*
* @param model the annotation model that should be refreshed
*/
public void refreshAnnotations(IAnnotationModel model) {
public void refreshAnnotations(final IAnnotationModel model) {

List<Annotation> annotationsToRemove = new ArrayList<Annotation>();
Map<Annotation, Position> annotationsToAdd = new HashMap<Annotation, Position>();
final List<Annotation> annotationsToRemove = new ArrayList<Annotation>();
final Map<Annotation, Position> annotationsToAdd = new HashMap<Annotation, Position>();

for (Iterator<Annotation> it = model.getAnnotationIterator(); it.hasNext(); ) {
for (final Iterator<Annotation> it = model.getAnnotationIterator(); it.hasNext(); ) {

Annotation annotation = it.next();
final Annotation annotation = it.next();

if (!(annotation instanceof ContributionAnnotation)) continue;

Position position = model.getPosition(annotation);
final Position position = model.getPosition(annotation);

if (position == null) {
log.warn("annotation could not be found in the current model: " + annotation);
Expand All @@ -157,9 +213,9 @@ public void refreshAnnotations(IAnnotationModel model) {
*/
annotationsToRemove.add(annotation);

ContributionAnnotation annotationToReplace = (ContributionAnnotation) annotation;
User source = annotationToReplace.getSource();
ContributionAnnotation annotationToAdd = new ContributionAnnotation(source, model);
final ContributionAnnotation annotationToReplace = (ContributionAnnotation) annotation;
final User source = annotationToReplace.getSource();
final ContributionAnnotation annotationToAdd = new ContributionAnnotation(source, model);

annotationsToAdd.put(annotationToAdd, position);

Expand All @@ -183,33 +239,6 @@ private ContributionAnnotationHistory getHistory(final User user) {
user, u -> new ContributionAnnotationHistory(MAX_HISTORY_LENGTH));
}

/**
* Add a contribution annotation to the annotation model and store it into the history of the
* associated user. Old entries are removed from the history and the annotation model.
*/
private void addContributionAnnotation(
final ContributionAnnotation annotation, final Position position) {

final ContributionAnnotationHistory history = getHistory(annotation.getSource());

final Pair<IAnnotationModel, List<ContributionAnnotation>> annotationToRemove =
history.removeHistoryEntry();

history.addNewEntry(annotation);

if (annotationToRemove == null) {
annotation.getModel().addAnnotation(annotation, position);
return;
}

// TODO OPTIMIZE it is usually 2 annotations after the queue is full
annotation.getModel().addAnnotation(annotation, position);

for (ContributionAnnotation a : annotationToRemove.getRight()) {
annotationToRemove.getLeft().removeAnnotation(a);
}
}

/**
* Removes all annotations from all annotation models that are currently stored in the history of
* all users.
Expand Down Expand Up @@ -239,8 +268,8 @@ private void removeFromHistoryAndAnnotationModel(
final Set<User> users = new HashSet<>();

for (final ContributionAnnotationHistory history : histories) {
List<ContributionAnnotation> allAnnotationsInHistory = history.clear();
for (ContributionAnnotation annotation : allAnnotationsInHistory) {
final List<ContributionAnnotation> allAnnotationsInHistory = history.clear();
for (final ContributionAnnotation annotation : allAnnotationsInHistory) {
annotationModels.add(annotation.getModel());
users.add(annotation.getSource());
}
Expand Down
Loading

0 comments on commit f411568

Please sign in to comment.