refact: improve GrammarRegistryManager

- simplifies code base - improves lookup of grammars via file extensions
eclipse-tm4e · Dec 22, 2023 · 0dd4241 · 0dd4241
1 parent 5bde2a6
commit 0dd4241
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 277 deletions.
diff --git a/...clipse.tm4e.registry/src/main/java/org/eclipse/tm4e/registry/IGrammarRegistryManager.java b/...clipse.tm4e.registry/src/main/java/org/eclipse/tm4e/registry/IGrammarRegistryManager.java
@@ -20,9 +20,7 @@
 import org.osgi.service.prefs.BackingStoreException;
 
 /**
- *
  * TextMate Grammar registry manager API.
- *
  */
 public interface IGrammarRegistryManager {
 
@@ -61,7 +59,7 @@ public interface IGrammarRegistryManager {
 	 *         has a grammar associated. Grammars associated with parent content-types will be returned if applicable.
 	 */
 	@Nullable
-	IGrammar getGrammarFor(IContentType @Nullable [] contentTypes);
+	IGrammar getGrammarFor(IContentType... contentTypes);
 
 	/**
 	 * @return the {@link IGrammar} for the given scope name and null otherwise.
@@ -70,7 +68,10 @@ public interface IGrammarRegistryManager {
 	IGrammar getGrammarForScope(String scopeName);
 
 	/**
-	 * @param fileType a file extension
+	 * <b>NOTE:</b> This method can be very expensive as it potentially results in eagerly loading of all registered grammar files,
+	 * therefore using {@link #getGrammarFor(IContentType...)} should be preferred.
+	 *
+	 * @param fileExtension a file extension
 	 *
 	 * @return the {@link IGrammar} for the file type name and null otherwise.
 	 */

diff --git a/...e.registry/src/main/java/org/eclipse/tm4e/registry/WorkingCopyGrammarRegistryManager.java b/...e.registry/src/main/java/org/eclipse/tm4e/registry/WorkingCopyGrammarRegistryManager.java
@@ -15,90 +15,72 @@
 import java.util.Collection;
 import java.util.List;
 
+import org.eclipse.core.runtime.content.IContentType;
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.tm4e.registry.internal.AbstractGrammarRegistryManager;
 import org.osgi.service.prefs.BackingStoreException;
 
 /**
- * Working copy of grammar registry manager.
+ * Working copy of grammar registry manager used by e.g. tm4e.ui/GrammarPreferencePage.
  */
 public class WorkingCopyGrammarRegistryManager extends AbstractGrammarRegistryManager {
 
 	private final IGrammarRegistryManager manager;
 
-	@Nullable
-	private List<IGrammarDefinition> added;
-
-	@Nullable
-	private List<IGrammarDefinition> removed;
+	private final List<IGrammarDefinition> added = new ArrayList<>();
+	private final List<IGrammarDefinition> removed = new ArrayList<>();
 
 	public WorkingCopyGrammarRegistryManager(final IGrammarRegistryManager manager) {
 		this.manager = manager;
-		load();
-	}
 
-	private void load() {
 		// Copy grammar definitions
-		final IGrammarDefinition[] definitions = manager.getDefinitions();
-		for (final IGrammarDefinition definition : definitions) {
+		for (final IGrammarDefinition definition : manager.getDefinitions()) {
 			super.registerGrammarDefinition(definition);
+		}
+	}
 
-			// Copy binding scope/content types
-			final String scopeName = definition.getScopeName();
-			final var contentTypes = manager.getContentTypesForScope(scopeName);
-			if (contentTypes != null) {
-				contentTypes.forEach(contentType -> super.registerContentTypeBinding(contentType, scopeName));
-			}
+	@Override
+	public @Nullable List<IContentType> getContentTypesForScope(String scopeName) {
+		return manager.getContentTypesForScope(scopeName);
+	}
 
-			// Copy injection
-			final Collection<String> injections = manager.getInjections(scopeName);
-			if (injections != null) {
-				for (final String injectFrom : injections) {
-					super.registerInjection(injectFrom, scopeName);
-				}
-			}
-		}
+	@Override
+	public @Nullable Collection<String> getInjections(String scopeName) {
+		return manager.getInjections(scopeName);
 	}
 
 	@Override
 	public void registerGrammarDefinition(final IGrammarDefinition definition) {
 		super.registerGrammarDefinition(definition);
-		var added = this.added;
-		if (added == null) {
-			added = this.added = new ArrayList<>();
-		}
 		added.add(definition);
 	}
 
 	@Override
 	public void unregisterGrammarDefinition(final IGrammarDefinition definition) {
 		super.unregisterGrammarDefinition(definition);
-		final var added = this.added;
-		if (added != null && added.contains(definition)) {
+		if (added.contains(definition)) {
 			added.remove(definition);
 		} else {
-			var removed = this.removed;
-			if (removed == null) {
-				removed = this.removed = new ArrayList<>();
-			}
 			removed.add(definition);
 		}
 	}
 
 	@Override
 	public void save() throws BackingStoreException {
-		if (added != null) {
+		if (!added.isEmpty()) {
 			for (final IGrammarDefinition definition : added) {
 				manager.registerGrammarDefinition(definition);
 			}
 		}
-		if (removed != null) {
+		if (!removed.isEmpty()) {
 			for (final IGrammarDefinition definition : removed) {
 				manager.unregisterGrammarDefinition(definition);
 			}
 		}
-		if (added != null || removed != null) {
+		if (!added.isEmpty() || removed.isEmpty()) {
 			manager.save();
 		}
+		added.clear();
+		removed.clear();
 	}
 }