Skip to content

[#446] Show symbols of external C/C++ files #449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 14, 2025
Merged

[#446] Show symbols of external C/C++ files #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b6e991e
[#446] support URI in symbols manager
ghentschke Mar 7, 2025
1bda2b6
remove unused code
ghentschke Mar 7, 2025
7c33765
close temporary loaded document
ghentschke Mar 7, 2025
2b9e7d2
use file from compileUnit
ghentschke Mar 7, 2025
afbd521
fix broken unit test
ghentschke Mar 7, 2025
b54eba4
increase service segment
ghentschke Mar 14, 2025
fe66aba
decrease service segment
ghentschke Mar 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...est/src/org/eclipse/cdt/lsp/test/internal/server/HasLanguageServerPropertyTesterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,16 @@ public void cleanUp() throws CoreException {
}

/**
* Tests whether LS enable test returns false for an external file when no (LSP) editor is opened.
* Tests whether LS enable test returns true for an external file when no (LSP) editor is opened.
* @throws IOException
*/
@Test
public void testLsNotEnabledForExternalFile_NoEditorOpen() throws CoreException, IOException {
//GIVEN is an external file which is not opened:
externalFile = new File(TEMP_DIR, EXTERNAL_HEADER_HPP);
//WHEN the file is not opened,
//THEN the hasLanguageServerPropertyTester.test returns FALSE for the given file URI:
assertFalse(new HasLanguageServerPropertyTester().test(externalFile.toURI(), null, null, null));
//THEN the hasLanguageServerPropertyTester.test returns TRUE for the given file URI:
assertTrue(new HasLanguageServerPropertyTester().test(externalFile.toURI(), null, null, null));
}

/**
Expand Down
3 changes: 2 additions & 1 deletion bundles/org.eclipse.cdt.lsp/META-INF/MANIFEST.MF
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bundle-Version: 3.0.100.qualifier

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must the service segment increased with every PR?

Copy link
Contributor

@jld01 jld01 Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, just once per release cycle: https://github.com/eclipse-platform/eclipse.platform/blob/master/docs/VersionNumbering.md

I now note that @jonahgraham has already merged version updates to main via #451 so there is no need to modify the version. Apologies for the confusion. Please do revert back to version 3.0.100 for this PR.

Copy link
Contributor Author

@ghentschke ghentschke Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jld01 if you have no objections I would like to merge this PR.

Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ Require-Bundle: org.eclipse.ui,
org.eclipse.cdt.debug.ui,
org.eclipse.ui.workbench.texteditor,
org.eclipse.tm4e.language_pack,
org.eclipse.jface.notifications
org.eclipse.jface.notifications,
org.eclipse.core.filesystem
Bundle-RequiredExecutionEnvironment: JavaSE-21
Automatic-Module-Name: org.eclipse.cdt.lsp
Bundle-ActivationPolicy: lazy
Expand Down
60 changes: 33 additions & 27 deletions ...les/org.eclipse.cdt.lsp/src/org/eclipse/cdt/lsp/internal/ui/navigator/SymbolsManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
import org.eclipse.core.filebuffers.FileBuffers;
import org.eclipse.core.filebuffers.IFileBuffer;
import org.eclipse.core.filebuffers.IFileBufferListener;
import org.eclipse.core.filebuffers.ITextFileBufferManager;
import org.eclipse.core.filebuffers.LocationKind;
import org.eclipse.core.filesystem.EFS;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
Expand Down Expand Up @@ -57,13 +59,15 @@ public class SymbolsManager implements IDeferredWorkbenchAdapter {

class CompileUnit {
public final IFile file;
public final URI uri;
public final SymbolsModel symbolsModel;
public volatile boolean isDirty = true;

public CompileUnit(IFile file) {
public CompileUnit(URI uri, IFile file) {
this.file = file;
this.uri = uri;
this.symbolsModel = new SymbolsModel();
this.symbolsModel.setUri(file.getLocationURI());
this.symbolsModel.setUri(uri);
}

public Object[] getElements() {
Expand Down Expand Up @@ -150,21 +154,17 @@ public ISchedulingRule getRule(Object object) {
}

public Object[] getTranslationUnitElements(ITranslationUnit translationUnit) {
if (translationUnit.getFile() != null) {
CompileUnit compileUnit = getCompileUnit(translationUnit.getFile().getLocationURI());
if (compileUnit != null) {
return compileUnit.getElements();
}
CompileUnit compileUnit = getCompileUnit(translationUnit.getLocationURI());
if (compileUnit != null) {
return compileUnit.getElements();
}
return null;
}

public boolean isDirty(ITranslationUnit translationUnit) {
if (translationUnit.getFile() != null) {
CompileUnit compileUnit = getCompileUnit(translationUnit.getFile().getLocationURI());
if (compileUnit != null) {
return compileUnit.isDirty;
}
CompileUnit compileUnit = getCompileUnit(translationUnit.getLocationURI());
if (compileUnit != null) {
return compileUnit.isDirty;
}
return true;
}
Expand Down Expand Up @@ -203,20 +203,12 @@ private CompileUnit getCompileUnit(URI key) {
}

private synchronized CompileUnit getCompileUnit(URI key, IFile file) {
if (file != null) {
return cachedSymbols.computeIfAbsent(key, uri -> new CompileUnit(file));
}
return cachedSymbols.get(key);
return cachedSymbols.computeIfAbsent(key, uri -> new CompileUnit(key, file));
}

private Object[] getCompileUnitElements(Object object) {
if (object instanceof ITranslationUnit unit) {
CompileUnit compileUnit = null;
if (unit.getFile() != null) {
compileUnit = getCompileUnit(unit.getFile().getLocationURI(), unit.getFile());
} else {
Platform.getLog(getClass()).error("Cannot fetch elements of translation unit " + unit.getElementName()); //$NON-NLS-1$
}
CompileUnit compileUnit = getCompileUnit(unit.getLocationURI(), unit.getFile());
if (compileUnit == null) {
return EMPTY;
}
Expand All @@ -236,6 +228,9 @@ private void refreshTreeContentFromLS(CompileUnit compileUnit) {
IDocument document = LSPEclipseUtils.getExistingDocument(compileUnit.file);
if (document == null) {
document = LSPEclipseUtils.getDocument(compileUnit.file);
if (document == null) {
document = LSPEclipseUtils.getDocument(compileUnit.uri);
}
temporaryLoadedDocument = true;
}
if (document != null) {
Expand Down Expand Up @@ -273,11 +268,22 @@ private void refreshTreeContentFromLS(CompileUnit compileUnit) {
if (temporaryLoadedDocument) {
//Note: the LS will be terminated via the shutdown command by LSP4E, when all documents have been disconnected.
//This is the case when no file is opened in the LSP based C/C++ editor.
try {
FileBuffers.getTextFileBufferManager().disconnect(compileUnit.file.getFullPath(),
LocationKind.IFILE, new NullProgressMonitor());
} catch (CoreException e) {
Platform.getLog(getClass()).error(e.getMessage(), e);
if (compileUnit.file != null) {
try {
FileBuffers.getTextFileBufferManager().disconnect(compileUnit.file.getFullPath(),
LocationKind.IFILE, new NullProgressMonitor());
} catch (CoreException e) {
Platform.getLog(getClass()).error(e.getMessage(), e);
}
} else {
try {
ITextFileBufferManager bufferManager = FileBuffers.getTextFileBufferManager();
if (bufferManager != null) {
bufferManager.disconnectFileStore(EFS.getStore(compileUnit.uri), new NullProgressMonitor());
}
} catch (CoreException e) {
Platform.getLog(getClass()).error(e.getMessage(), e);
}
}
}
lock.unlock();
Expand Down
4 changes: 2 additions & 2 deletions bundles/org.eclipse.cdt.lsp/src/org/eclipse/cdt/lsp/util/LspUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public static boolean isFileOpenedInLspEditor(URI uri) {
// the file has not been opened yet -> goto definition/declaration case
return isLspEditorActive();
}
return false;
return true; // we're lazy here
}

public static Map<Integer, URI> getFilesInLspBasedEditor() {
Expand Down Expand Up @@ -154,7 +154,7 @@ private static boolean isLspEditorActive() {
return LspPlugin.LSP_C_EDITOR_ID.equals(activeEditor.getEditorSite().getId());
}
}
return false;
return true; // activeWorkbenchWindow or active page is null. We've been called probably from a non UI thread. This occurs when LSP4E searches for valid LS. It's fine to return true here.
}

public static boolean checkForCContentType(IEditorInput editorInput) {
Expand Down
Loading