Skip to content
Closed
Changes from all commits
Commits
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
120 changes: 113 additions & 7 deletions src/dcd/server/autocomplete/util.d
Original file line number Diff line number Diff line change
Expand Up @@ -755,14 +755,120 @@ unittest

AutocompleteResponse.Completion makeSymbolCompletionInfo(const DSymbol* symbol, char kind)
{
string definition;
if ((kind == CompletionKind.variableName || kind == CompletionKind.memberVariableName) && symbol.type)
definition = symbol.type.name ~ ' ' ~ symbol.name;
else if (kind == CompletionKind.enumMember)
definition = symbol.name; // TODO: add enum value to definition string
string definition = symbol.callTip;
string symbolName = symbol.name;
char completionKind = kind;

// if symbol has a type, then let's try to resolve it
// and properly format the definition
if (symbol.type)
{
DSymbol* sym = cast(DSymbol*) symbol;
DSymbol* symType = sym.type;

while (symType && sym && sym.type != sym)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should have a counter / max limit just in case there are infinite cycles to break after e.g. 50 iterations or so

{
// if that's a function, and its type is an alias then stop
// otherwise it breaks aliases of local functions
if (sym.kind == CompletionKind.functionName && symType.kind == CompletionKind.aliasName)
break;

// otherwise if we resolved something that's not an alias, we can stop
else if (sym.kind != CompletionKind.aliasName && symType.kind != CompletionKind.aliasName)
break;

// dummy are internal DCD stuff, as per the documentation on the enum
// we can safely ignore it
if (symType.kind == CompletionKind.dummy) break;

if (!symType.type) break;

sym = symType;
symType = symType.type;
}

if (sym)
{
// if the symbol has a calltip, then let's use that
// otherwise build something with its type
if (sym.callTip.length > 0 && indexOf(sym.callTip, " ") != -1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (sym.callTip.length > 0 && indexOf(sym.callTip, " ") != -1)
if (sym.callTip.length > 0 && sym.callTip.canFind(" "))

canFind is more readable than indexOf() == -1

{
definition = sym.callTip;
}
else if (sym.type)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
else if (sym.type)
else if (symType)

{
string symName = symbol.name;
string symTypeName = symType.callTip.length > 0 ? symType.callTip : symType.name;
definition = symTypeName ~ " " ~ symName;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you seem to be discarding sym.callTip here if it has content, but no whitespace



// if that's a function, and definition doesn't have white space
// that mean the function returns either auto/enum
if (sym.kind == CompletionKind.functionName && indexOf(definition, " ") == -1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (sym.kind == CompletionKind.functionName && indexOf(definition, " ") == -1)
if (sym.kind == CompletionKind.functionName && !definition.canFind(" "))

{
definition = "auto " ~ definition;
}
Comment on lines +806 to +811
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this whole check might be trying to be too smart, just checking for a space is bound to have issues further down the line e.g. with templates/template arguments that have spaces in them.

I would suggest removing this special case for now to keep the diff smaller, this looks to me rather like something that needs to be changed on dsymbol to be more effective.



// if that's an alias that points to a function,
// then set the proper completion kind
// and show what it resolves to
if (symbol.kind == CompletionKind.aliasName && sym.kind == CompletionKind.functionName)
{
//completionKind = sym.kind;
definition = "-> " ~ sym.name ~ " : " ~ definition;
}
}
}
else
definition = symbol.callTip;
{
switch (kind) with (CompletionKind)
{
case enumMember:
definition = symbol.name; // TODO: add enum value to definition string
break;
case className:
definition = "Class";
break;
case interfaceName:
definition = "Interface";
break;
case structName:
definition = "Struct";
break;
case unionName:
definition = "Union";
break;
case keyword:
definition = "Keyword";
break;
case enumName:
definition = "Enum";
break;
case packageName:
definition = "Package";
break;
case moduleName:
definition = "Module";
break;
case templateName:
case mixinTemplateName:
definition = "Template";
break;
case typeTmpParam:
definition = "<T>";
break;
case variadicTmpParam:
definition = "<T...>";
break;
case aliasName: // Alias (eventually should show what it aliases to)
default:
break;
}
}

// TODO: definition strings could include more information, like on classes inheritance
return AutocompleteResponse.Completion(symbol.name, kind, definition,
return AutocompleteResponse.Completion(symbolName, completionKind, definition,
symbol.symbolFile, symbol.location, symbol.doc);
}