Skip to content

Commit 7d3167a

Browse files
sebthomsebthom
committed
fix(preferences): JS/TS settings now apply without restart
Previously only workspace-wide servers were notified (forProject(null)). Now also notify per-project and per-document servers (open projects/editors). Ensures TypeScript/JavaScript formatter and related options take effect immediately.
1 parent 25bfa23 commit 7d3167a

File tree

4 files changed

+258
-11
lines changed

4 files changed

+258
-11
lines changed

‎org.eclipse.wildwebdeveloper.tests/src/org/eclipse/wildwebdeveloper/tests/TestTypeScriptFormattingPreferences.java‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
*
4+
* This program and the accompanying materials are made
5+
* available under the terms of the Eclipse License 2.0
6+
* which is available at https://www.eclipse.org/legal/epl-2.0/
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
*
10+
* Contributors:
11+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation
12+
*******************************************************************************/
13+
package org.eclipse.wildwebdeveloper.tests;
14+
15+
import static org.junit.jupiter.api.Assertions.*;
16+
17+
import java.util.ArrayList;
18+
import java.util.Comparator;
19+
import java.util.List;
20+
import java.util.concurrent.CompletableFuture;
21+
import java.util.concurrent.TimeUnit;
22+
import java.util.function.Supplier;
23+
24+
import org.eclipse.core.resources.IFile;
25+
import org.eclipse.core.resources.IProject;
26+
import org.eclipse.core.resources.ResourcesPlugin;
27+
import org.eclipse.jface.text.BadLocationException;
28+
import org.eclipse.jface.text.Document;
29+
import org.eclipse.jface.text.IDocument;
30+
import org.eclipse.lsp4e.LSPEclipseUtils;
31+
import org.eclipse.lsp4e.LanguageServers;
32+
import org.eclipse.lsp4e.LanguageServersRegistry;
33+
import org.eclipse.lsp4e.LanguageServersRegistry.LanguageServerDefinition;
34+
import org.eclipse.lsp4j.DocumentFormattingParams;
35+
import org.eclipse.lsp4j.FormattingOptions;
36+
import org.eclipse.lsp4j.Range;
37+
import org.eclipse.lsp4j.ServerCapabilities;
38+
import org.eclipse.lsp4j.TextDocumentIdentifier;
39+
import org.eclipse.lsp4j.TextEdit;
40+
import org.eclipse.lsp4j.services.LanguageServer;
41+
import org.eclipse.ui.PlatformUI;
42+
import org.eclipse.ui.ide.IDE;
43+
import org.eclipse.ui.tests.harness.util.DisplayHelper;
44+
import org.eclipse.ui.texteditor.AbstractTextEditor;
45+
import org.eclipse.wildwebdeveloper.Activator;
46+
import org.eclipse.wildwebdeveloper.jsts.JSTSLanguageServer;
47+
import org.eclipse.wildwebdeveloper.jsts.ui.preferences.JSTSLanguagePreferences;
48+
import org.junit.jupiter.api.AfterEach;
49+
import org.junit.jupiter.api.BeforeEach;
50+
import org.junit.jupiter.api.Test;
51+
import org.junit.jupiter.api.extension.ExtendWith;
52+
53+
@ExtendWith(AllCleanRule.class)
54+
class TestTypeScriptFormattingPreferences {
55+
56+
private IProject project;
57+
58+
@BeforeEach
59+
void setUpProject() throws Exception {
60+
this.project = ResourcesPlugin.getWorkspace().getRoot().getProject(getClass().getName() + System.nanoTime());
61+
project.create(null);
62+
project.open(null);
63+
}
64+
65+
@AfterEach
66+
void tearDown() {
67+
// Reset preference toggled by this test to avoid cross-test interference
68+
Activator.getDefault().getPreferenceStore().setValue(JSTSLanguagePreferences.TS.format_indentSize, 4);
69+
}
70+
71+
@Test
72+
void testFormatterPrefsApplyWithoutRestart() throws Exception {
73+
// Arrange: create a TS file with a function declaration where spacing can change
74+
IFile file = project.getFile("format.ts");
75+
String original = "function greet() {\n return 1\n}\n";
76+
file.create(original.getBytes("UTF-8"), true, false, null);
77+
78+
var editor = (AbstractTextEditor) IDE.openEditor(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage(), file);
79+
IDocument document = LSPEclipseUtils.getDocument(editor);
80+
81+
// Give the LS time to initialize and connect to the document
82+
DisplayHelper.sleep(2000);
83+
84+
// Acquire the running JS/TS language server for this document (filter strictly by definition id)
85+
LanguageServerDefinition jstsDef = LanguageServersRegistry.getInstance().getDefinition(JSTSLanguageServer.JSTS_LANGUAGE_SERVER_ID);
86+
var servers = new ArrayList<LanguageServer>();
87+
LanguageServers.forDocument(document)
88+
.withCapability(ServerCapabilities::getDocumentFormattingProvider)
89+
.collectAll((wrapper, ls) -> CompletableFuture.completedFuture(
90+
jstsDef.equals(wrapper.serverDefinition) ? ls : null))
91+
.thenAccept(servers::addAll)
92+
.get(5, TimeUnit.SECONDS);
93+
assertFalse(servers.isEmpty(), "Expected running JSTS server for document");
94+
LanguageServer jsts = servers.get(0);
95+
96+
// Helper to request formatting and return the formatted text without mutating the editor document
97+
Supplier<String> formatWithCurrentPrefs = () -> {
98+
try {
99+
var opts = new FormattingOptions(4, true);
100+
var params = new DocumentFormattingParams(
101+
new TextDocumentIdentifier(LSPEclipseUtils.toUri(document).toString()), opts);
102+
List<? extends TextEdit> edits = jsts.getTextDocumentService().formatting(params).get(10, TimeUnit.SECONDS);
103+
return applyTextEdits(original, edits);
104+
} catch (Exception e) {
105+
throw new RuntimeException(e);
106+
}
107+
};
108+
109+
// Act 1: format with default prefs (indentSize=4)
110+
String formattedIndent4 = formatWithCurrentPrefs.get();
111+
int indent4 = leadingSpacesOfLineStartingWith(formattedIndent4, "return 1");
112+
assertTrue(indent4 >= 0, "Could not locate return line in formatted output");
113+
114+
// Change preference: typescript.format.indentSize = 2
115+
Activator.getDefault().getPreferenceStore().setValue(JSTSLanguagePreferences.TS.format_indentSize, 2);
116+
DisplayHelper.sleep(1500); // Allow async preference broadcast
117+
118+
String formattedIndent2 = formatWithCurrentPrefs.get();
119+
int indent2 = leadingSpacesOfLineStartingWith(formattedIndent2, "return 1");
120+
assertTrue(indent2 >= 0, "Could not locate return line after indentSize=2 change");
121+
122+
// Change preference again: typescript.format.indentSize = 6
123+
Activator.getDefault().getPreferenceStore().setValue(JSTSLanguagePreferences.TS.format_indentSize, 6);
124+
DisplayHelper.sleep(1500);
125+
126+
String formattedIndent6 = formatWithCurrentPrefs.get();
127+
int indent6 = leadingSpacesOfLineStartingWith(formattedIndent6, "return 1");
128+
assertTrue(indent6 >= 0, "Could not locate return line after indentSize=6 change");
129+
130+
// Assertions: indentation should reflect the configured indent size without server restart
131+
assertNotEquals(indent4, indent2, "Indent should change when indentSize changes to 2");
132+
assertNotEquals(indent2, indent6, "Indent should change when indentSize changes to 6");
133+
assertTrue(indent6 > indent2, () -> "Expected indent6(" + indent6 + ") > indent2(" + indent2 + ")");
134+
}
135+
136+
private static String applyTextEdits(String original, List<? extends TextEdit> edits) {
137+
if (edits == null || edits.isEmpty()) {
138+
return original;
139+
}
140+
var doc = new Document(original);
141+
// Apply edits from bottom to top to keep offsets valid
142+
edits.stream().sorted(Comparator
143+
.comparing((TextEdit e) -> e.getRange().getStart().getLine())
144+
.thenComparing(e -> e.getRange().getStart().getCharacter())
145+
.reversed())
146+
.forEach(e -> replace(doc, e.getRange(), e.getNewText()));
147+
return doc.get();
148+
}
149+
150+
private static void replace(IDocument doc, Range range, String newText) {
151+
try {
152+
int start = LSPEclipseUtils.toOffset(range.getStart(), doc);
153+
int end = LSPEclipseUtils.toOffset(range.getEnd(), doc);
154+
doc.replace(start, end - start, newText);
155+
} catch (BadLocationException e) {
156+
throw new RuntimeException(e);
157+
}
158+
}
159+
160+
private static int leadingSpacesOfLineStartingWith(String text, String token) {
161+
String[] lines = text.split("\r?\n");
162+
for (String line : lines) {
163+
int i = 0;
164+
while (i < line.length() && line.charAt(i) == ' ')
165+
i++;
166+
if (line.substring(i).startsWith(token)) {
167+
return i;
168+
}
169+
}
170+
return -1;
171+
}
172+
}

‎org.eclipse.wildwebdeveloper/META-INF/MANIFEST.MF‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,6 @@ Export-Package: org.eclipse.wildwebdeveloper;x-friends:="org.eclipse.wildwebdeve
3939
org.eclipse.wildwebdeveloper.debug;x-internal:=true,
4040
org.eclipse.wildwebdeveloper.debug.node;x-internal:=true,
4141
org.eclipse.wildwebdeveloper.debug.npm;x-internal:=true,
42-
org.eclipse.wildwebdeveloper.markdown;x-friends:="org.eclipse.wildwebdeveloper.tests"
42+
org.eclipse.wildwebdeveloper.markdown;x-friends:="org.eclipse.wildwebdeveloper.tests",
43+
org.eclipse.wildwebdeveloper.jsts;x-friends:="org.eclipse.wildwebdeveloper.tests",
44+
org.eclipse.wildwebdeveloper.jsts.ui.preferences;x-friends:="org.eclipse.wildwebdeveloper.tests"

‎org.eclipse.wildwebdeveloper/src/org/eclipse/wildwebdeveloper/jsts/JSTSLanguageServer.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@
3939

4040
public class JSTSLanguageServer extends ProcessStreamConnectionProviderWithPreference {
4141

42-
private static final String JSTS_LANGUAGE_SERVER_ID = "org.eclipse.wildwebdeveloper.jsts";
42+
public static final String JSTS_LANGUAGE_SERVER_ID = "org.eclipse.wildwebdeveloper.jsts";
4343

4444
private static final String[] SUPPORTED_SECTIONS = { "typescript", "javascript" };
4545

4646
private static String tsserverPath;
47-
47+
4848
public JSTSLanguageServer() {
4949
super(JSTS_LANGUAGE_SERVER_ID, Activator.getDefault().getPreferenceStore(), SUPPORTED_SECTIONS);
5050
List<String> commands = new ArrayList<>();
@@ -77,7 +77,7 @@ public Object getInitializationOptions(URI rootUri) {
7777
plugins.add(new TypeScriptPlugin("typescript-lit-html-plugin"));
7878
plugins.add(new TypeScriptPlugin("@vue/typescript-plugin", "@vue/language-server", new String[] {"vue"}));
7979
options.put("plugins", plugins.stream().map(TypeScriptPlugin::toMap).toArray());
80-
80+
8181
// If the tsserver path is not explicitly specified, tsserver will use the local
8282
// TypeScript version installed as part of the project's dependencies, if found.
8383
if (!TYPESCRIPT_PREFERENCES_TSSERVER_TYPESCRIPT_VERSION_PROJECT.equals(getTypeScriptVersion())) {
@@ -93,11 +93,11 @@ public Object getInitializationOptions(URI rootUri) {
9393
if (maxTsServerMemory != null) {
9494
options.put("maxTsServerMemory", maxTsServerMemory);
9595
}
96-
96+
9797
options.put("disableAutomaticTypingAcquisition", true); // not working on mac/linux
9898
options.put("hostInfo", "Eclipse");
9999
options.put("watchOptions", new HashMap<>());
100-
100+
101101
return options;
102102
}
103103

‎org.eclipse.wildwebdeveloper/src/org/eclipse/wildwebdeveloper/ui/preferences/ProcessStreamConnectionProviderWithPreference.java‎

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* SPDX-License-Identifier: EPL-2.0
99
*
1010
* Contributors:
11-
* Angelo ZERR (Red Hat Inc.) - initial implementation
11+
* Angelo ZERR (Red Hat Inc.) - initial implementation
1212
*******************************************************************************/
1313
package org.eclipse.wildwebdeveloper.ui.preferences;
1414

@@ -18,24 +18,36 @@
1818
import java.util.HashMap;
1919
import java.util.Map;
2020
import java.util.Objects;
21+
import java.util.Set;
2122
import java.util.concurrent.CompletableFuture;
23+
import java.util.concurrent.ConcurrentHashMap;
2224

25+
import org.eclipse.core.resources.IProject;
26+
import org.eclipse.core.resources.ResourcesPlugin;
2327
import org.eclipse.jface.preference.IPreferenceStore;
28+
import org.eclipse.jface.text.IDocument;
2429
import org.eclipse.jface.util.IPropertyChangeListener;
2530
import org.eclipse.jface.util.PropertyChangeEvent;
31+
import org.eclipse.lsp4e.LSPEclipseUtils;
2632
import org.eclipse.lsp4e.LanguageServers;
2733
import org.eclipse.lsp4e.LanguageServersRegistry;
2834
import org.eclipse.lsp4e.LanguageServersRegistry.LanguageServerDefinition;
2935
import org.eclipse.lsp4e.server.ProcessStreamConnectionProvider;
3036
import org.eclipse.lsp4j.DidChangeConfigurationParams;
37+
import org.eclipse.lsp4j.services.LanguageServer;
3138
import org.eclipse.lsp4j.services.WorkspaceService;
39+
import org.eclipse.ui.IEditorPart;
40+
import org.eclipse.ui.IEditorReference;
41+
import org.eclipse.ui.IWorkbenchPage;
42+
import org.eclipse.ui.IWorkbenchWindow;
43+
import org.eclipse.ui.PlatformUI;
3244

3345
/**
3446
* This class extends {@link ProcessStreamConnectionProvider} to manage
3547
* {@link IPreferenceStore} and call
3648
* {@link WorkspaceService#didChangeConfiguration(DidChangeConfigurationParams)}
3749
* when the preference store changes.
38-
*
50+
*
3951
*/
4052
public abstract class ProcessStreamConnectionProviderWithPreference extends ProcessStreamConnectionProvider
4153
implements IPropertyChangeListener {
@@ -158,13 +170,74 @@ private void removePropertyChangeListenerIfNeed() {
158170
@Override
159171
public void propertyChange(PropertyChangeEvent event) {
160172
if (isAffected(event)) {
161-
LanguageServerDefinition languageServerDefinition = getLanguageServerDefinition();
173+
LanguageServerDefinition languageServerDefinition = getLanguageServerDefinition();
162174
@SuppressWarnings("rawtypes")
163175
DidChangeConfigurationParams params = new DidChangeConfigurationParams(createSettings());
164176

177+
/*
178+
* Fan-out strategy and rationale
179+
* --------------------------------
180+
* We must deliver didChangeConfiguration to every running instance of the
181+
* language server for this provider's id, regardless of how LSP4E can find it.
182+
* LSP4E discovery varies by scope, so we notify in 3 passes and de-duplicate:
183+
* 1) Workspace-wide (null project): catches singleton or workspace-folder-aware servers.
184+
* 2) Per-project: picks up per-project servers that don't expose workspace folders (eg JSTS).
185+
* 3) Per-document (open editors): covers files outside the workspace or not yet tied to a project.
186+
*
187+
* Note: withPreferredServer(...) only reorders candidates; it does not filter.
188+
* We therefore compare wrapper.serverDefinition for equality and track already
189+
* notified LanguageServer proxies to avoid duplicate notifications across scopes.
190+
* excludeInactive() is intentional to avoid starting servers as a side-effect
191+
* of a preference change.
192+
*/
193+
194+
final Set<LanguageServer> notifiedServers = ConcurrentHashMap.newKeySet();
195+
196+
// 1) Workspace-wide: singleton or workspace-folder-aware servers
165197
LanguageServers.forProject(null).withPreferredServer(languageServerDefinition).excludeInactive()
166-
.collectAll((w, ls) -> CompletableFuture.completedFuture(ls)).thenAccept(
167-
lss -> lss.stream().forEach(ls -> ls.getWorkspaceService().didChangeConfiguration(params)));
198+
.collectAll((wrapper, server) -> {
199+
if (languageServerDefinition.equals(wrapper.serverDefinition) && notifiedServers.add(server)) {
200+
server.getWorkspaceService().didChangeConfiguration(params);
201+
}
202+
return CompletableFuture.completedFuture(null);
203+
});
204+
205+
// 2) Per-project: include servers that don't support workspace folders (they won't be returned by forProject(null))
206+
for (final IProject project : ResourcesPlugin.getWorkspace().getRoot().getProjects()) {
207+
if (!project.isOpen())
208+
continue;
209+
210+
LanguageServers.forProject(project).withPreferredServer(languageServerDefinition).excludeInactive()
211+
.collectAll((wrapper, server) -> {
212+
if (languageServerDefinition.equals(wrapper.serverDefinition) && notifiedServers.add(server)) {
213+
server.getWorkspaceService().didChangeConfiguration(params);
214+
}
215+
return CompletableFuture.completedFuture(null);
216+
});
217+
}
218+
219+
// 3) Per-document: open editors (covers external files or untied docs)
220+
for (final IWorkbenchWindow win : PlatformUI.getWorkbench().getWorkbenchWindows()) {
221+
for (final IWorkbenchPage page : win.getPages()) {
222+
for (final IEditorReference ref : page.getEditorReferences()) {
223+
final IEditorPart editor = ref.getEditor(false); // do not restore unopened editors
224+
if (editor == null)
225+
continue;
226+
227+
final IDocument doc = LSPEclipseUtils.getDocument(editor.getEditorInput());
228+
if (doc == null)
229+
continue;
230+
231+
LanguageServers.forDocument(doc).withPreferredServer(languageServerDefinition)
232+
.collectAll((wrapper, server) -> {
233+
if (languageServerDefinition.equals(wrapper.serverDefinition) && notifiedServers.add(server)) {
234+
server.getWorkspaceService().didChangeConfiguration(params);
235+
}
236+
return CompletableFuture.completedFuture(null);
237+
});
238+
}
239+
}
240+
}
168241
}
169242
}
170243

0 commit comments

Comments
 (0)