Add ability to find symbols by name #29
Conversation
literalpie
force-pushed
the
find-symbols-by-name
branch
from
fa6a230
to
c281bec
Compare
|
I've created a (WIP) branch in the sourcekit-lsp project with that side of this change. |
This can be used in lsp projects to find all symbols in a workspace that match a string. https://bugs.swift.org/browse/SR-10806
literalpie
force-pushed
the
find-symbols-by-name
branch
from
c281bec
to
313cc12
Compare
| @@ -80,6 +80,49 @@ public final class IndexStoreDB { | |||
| return body(SymbolOccurrence(occur)) | |||
| } | |||
| } | |||
|
|
|||
| public func forEachCanonicalSymbolOccurrenceByName(symbolName: String, body: @escaping (SymbolOccurrence) -> Bool) { | |||
There was a problem hiding this comment.
I suggest the name forEachCanonicalSymbolOccurrence(byName:body:)
There was a problem hiding this comment.
It should also return Bool and have @discardableResult like the other functions. Same with forEachCanonicalSymbolOccurrence.
| /// @param index an IndexStoreDB object which contains the symbols | ||
| /// @param receiver a function to be called for each symbol, the CString of the symbol will be passed in to this function. | ||
| /// The function should return a boolean indicating whether the looping should continue. | ||
| bool |
There was a problem hiding this comment.
The documentation comments should go in the header, not the .cpp file. Please capitalize the first word as well. Same for the other functions.
| } | ||
| } | ||
|
|
||
| public func findSymbols(matching query: String) -> [SymbolOccurrence] { |
There was a problem hiding this comment.
This method seems like it's fairly specific to what you want to do in sourcekit-lsp and the name doesn't make it clear what it's doing (e.g. skipping system occurrences?). I suggest we remove it from the index API and just call forEachCanonicalSymbolOccurrence(containing:...) from sourcekit-lsp.
lib/CIndexStoreDB/CIndexStoreDB.cpp
Outdated
| bool | ||
| indexstoredb_for_each_canonical_symbol_occurence_containing_pattern( | ||
| indexstoredb_index_t index, | ||
| const char *_Nonnull Pattern, |
There was a problem hiding this comment.
For new C++ code, let's stick to lowerCamelCase names in variables. This applies to the other function parameters as well.
| indexstoredb_for_each_symbol_name(_Nonnull indexstoredb_index_t index, _Nonnull indexstoredb_symbol_name_receiver); | ||
|
|
||
| INDEXSTOREDB_PUBLIC bool | ||
| indexstoredb_for_each_canonical_symbol_occurence_containing_pattern( |
There was a problem hiding this comment.
To be consistent with the other symbol names, I suggest the following:
indexstoredb_index_symbol_namesindexstoredb_index_canonical_symbol_occurences_containing_patternindexstoredb_index_canonical_symbol_occurences_by_name
We've been using "index_" in the name since these are effectively methods of the index object, and I've been using plurals instead of "for_each". I'm not wed to these names, but it would be good to be consistent.
Add discardableResult to new functions. Return Bool in new functions. Make all new variables lowercase. Rename functions to match existing patterns.
|
I believe I've addressed all of the comments again. Thank you again for the feedback! |
|
@swift-ci Please test |
This can be used in lsp projects to find
all symbols in a workspace that match a string.
https://bugs.swift.org/browse/SR-10806