Implement label details using new method

4 files changed, 76 insertions, 108 deletions

diff --git a/src/server/analysis.odin b/src/server/analysis.odin
index 48d2452..9c2315e 100644
--- a/src/server/analysis.odin
+++ b/src/server/analysis.odin
@@ -891,7 +891,7 @@ resolve_basic_lit :: proc(ast_context: ^AstContext, basic_lit: ast.Basic_Lit) ->
 	out commented because of an infinite loop in parse_f64
 	else if v, ok := strconv.parse_f64(basic_lit.tok.text); ok {
 		value.type = .Float
-	} 
+	}
 	*/
 
 	symbol.pkg = ast_context.current_package
@@ -922,7 +922,10 @@ resolve_basic_directive :: proc(
 // Gets the return type of the proc.
 // Requires the underlying call expression to handle some builtin procs
 get_proc_return_types :: proc(
-	ast_context: ^AstContext, symbol: Symbol, call: ^ast.Call_Expr, is_mutable: bool,
+	ast_context: ^AstContext,
+	symbol: Symbol,
+	call: ^ast.Call_Expr,
+	is_mutable: bool,
 ) -> []^ast.Expr {
 	return_types := make([dynamic]^ast.Expr, context.temp_allocator)
 	if ret, ok := check_builtin_proc_return_type(symbol, call, is_mutable); ok {
@@ -1028,7 +1031,7 @@ check_builtin_proc_return_type :: proc(symbol: Symbol, call: ^ast.Call_Expr, is_
 			}
 			if curr_candidate != nil {
 				return convert_candidate(curr_candidate, is_mutable), true
-	 		}
+			}
 		case "abs":
 			for arg in call.args {
 				if lit, _, ok := get_basic_lit_value(arg); ok {
@@ -1757,12 +1760,9 @@ internal_resolve_type_identifier :: proc(ast_context: ^AstContext, node: ast.Ide
 			}
 
 			if return_symbol, ok = internal_resolve_type_expression(ast_context, v.expr); ok {
-				return_types := get_proc_return_types(ast_context, return_symbol, v, global.mutable);
+				return_types := get_proc_return_types(ast_context, return_symbol, v, global.mutable)
 				if len(return_types) > 0 {
-					return_symbol, ok = internal_resolve_type_expression(
-						ast_context,
-						return_types[0],
-					)
+					return_symbol, ok = internal_resolve_type_expression(ast_context, return_types[0])
 				}
 				// Otherwise should be a parapoly style
 			}
@@ -3601,11 +3601,11 @@ get_locals_block_stmt :: proc(
 	ast_context: ^AstContext,
 	document_position: ^DocumentPositionContext,
 ) {
-	/* 
+	/*
 	   We need to handle blocks for non mutable and mutable: non mutable has no order for their value declarations, except for nested blocks where they are hidden by scope
-	   For non_mutable_only we set the document_position.position to be the end of the function to get all the non-mutable locals, but that shouldn't apply to the nested block itself, 
+	   For non_mutable_only we set the document_position.position to be the end of the function to get all the non-mutable locals, but that shouldn't apply to the nested block itself,
 	   but will for it's content.
-	   
+
 	   Therefore we use nested_position that is the exact token we are interested in.
 
 	   Example:
diff --git a/src/server/completion.odin b/src/server/completion.odin
index ed27270..63a22ab 100644
--- a/src/server/completion.odin
+++ b/src/server/completion.odin
@@ -173,14 +173,6 @@ get_completion_list :: proc(
 	items := convert_completion_results(&ast_context, &position_context, results[:], completion_type)
 	list.items = items
 	list.isIncomplete = is_incomplete
-
-	// For now we simply convert all the docs to markdown here.
-	//convert_docs_to_markdown(&list)
-	//
-	//if common.config.enable_label_details {
-	//	format_to_label_details(&list)
-	//}
-	//
 	return list, true
 }
 
@@ -207,7 +199,7 @@ convert_completion_results :: proc(
 		if item, ok := result.completion_item.?; ok {
 			if common.config.enable_label_details {
 				item.labelDetails = CompletionItemLabelDetails {
-					detail = item.detail,
+					description = item.detail,
 				}
 			}
 			// temporary as we move things to use the symbols directly
@@ -258,14 +250,23 @@ convert_completion_results :: proc(
 			documentation = write_hover_content(ast_context, result.symbol)
 		}
 		if common.config.enable_label_details {
-			// TODO: compute
+			// detail      = left
+			// description = right
 			details := CompletionItemLabelDetails{}
-
 			if result.detail != "" {
-				details.detail = result.detail
+				details.description = result.detail
 			} else {
-				// TODO: this should be a better function for showing what it wants?
-				details.detail = get_short_signature(ast_context, &result.symbol)
+				details.detail = get_completion_details(ast_context, result.symbol)
+				details.description = get_completion_description(ast_context, result.symbol)
+			}
+			// hack for sublime text's issue
+			// remove when this issue is fixed: https://github.com/sublimehq/sublime_text/issues/6033
+			// or if this PR gets merged: https://github.com/sublimelsp/LSP/pull/2293
+			if common.config.client_name == "Sublime Text LSP" {
+				if strings.contains(details.detail, "..") && strings.contains(details.detail, "#") {
+					s, _ := strings.replace_all(details.detail, "..", "ꓸꓸ", allocator = context.temp_allocator)
+					details.detail = s
+				}
 			}
 			item.labelDetails = details
 		}
@@ -291,6 +292,28 @@ convert_completion_results :: proc(
 	return items[:]
 }
 
+get_completion_details :: proc(ast_context: ^AstContext, symbol: Symbol) -> string {
+	#partial switch v in symbol.value {
+	case SymbolProcedureValue:
+		sb := strings.builder_make(ast_context.allocator)
+		write_proc_param_list_and_return(&sb, v)
+		return strings.to_string(sb)
+	case SymbolAggregateValue:
+		return "(..)"
+	}
+	return ""
+}
+
+get_completion_description :: proc(ast_context: ^AstContext, symbol: Symbol) -> string {
+	#partial switch v in symbol.value {
+	case SymbolProcedureValue:
+		return ""
+	case SymbolAggregateValue:
+		return ""
+	}
+	return get_short_signature(ast_context, symbol)
+}
+
 get_attribute_completion :: proc(
 	ast_context: ^AstContext,
 	position_context: ^DocumentPositionContext,
@@ -2101,68 +2124,6 @@ append_magic_union_completion :: proc(
 
 }
 
-//Temporary hack to support labeldetails
-format_to_label_details :: proc(list: ^CompletionList) {
-	// detail      = left
-	// description = right
-
-	for &item in list.items {
-		// log.errorf("item:%v: %v:%v", item.kind, item.label, item.detail)
-		#partial switch item.kind {
-		case .Function:
-			proc_info := ""
-			// Split the leading name of the proc
-			proc_info_split := strings.split_n(item.detail, " proc", 2)
-			if len(proc_info_split) == 1 {
-				// No proc declaration (eg for a proc group)
-				proc_info = "(..)"
-			} else if len(proc_info_split) == 2 {
-				proc_info = proc_info_split[1]
-			}
-
-			item.labelDetails = CompletionItemLabelDetails {
-				detail      = proc_info,
-				description = "",
-			}
-		case .Variable, .Constant, .Field:
-			type_index := strings.index(item.detail, ":")
-			item.labelDetails = CompletionItemLabelDetails {
-				detail      = "",
-				description = item.detail[type_index + 1:],
-			}
-		case .Struct, .Enum, .Class:
-			type_index := strings.index(item.detail, ":")
-			item.labelDetails = CompletionItemLabelDetails {
-				detail      = "",
-				description = item.detail[type_index + 1:],
-			}
-		}
-
-		// hack for sublime text's issue
-		// remove when this issue is fixed: https://github.com/sublimehq/sublime_text/issues/6033
-		// or if this PR gets merged: https://github.com/sublimelsp/LSP/pull/2293
-		if common.config.client_name == "Sublime Text LSP" {
-			dt := &item.labelDetails.? or_else nil
-			if dt == nil do continue
-			if strings.contains(dt.detail, "..") && strings.contains(dt.detail, "#") {
-				s, _ := strings.replace_all(dt.detail, "..", "ꓸꓸ", allocator = context.temp_allocator)
-				dt.detail = s
-			}
-		}
-	}
-}
-
-convert_docs_to_markdown :: proc(list: ^CompletionList) {
-	for &item in list.items {
-		if s, ok := item.documentation.(string); ok {
-			item.documentation = MarkupContent {
-				kind = "markdown",
-				value = s,
-			}
-		}
-	}
-}
-
 bitset_operators: map[string]bool = {
 	"|"  = true,
 	"&"  = true,
diff --git a/src/server/documentation.odin b/src/server/documentation.odin
index fa6eff5..9767a9d 100644
--- a/src/server/documentation.odin
+++ b/src/server/documentation.odin
@@ -107,9 +107,9 @@ keywords_docs: map[string]bool = {
 // Adds signature and docs information to the provided symbol
 build_documentation :: proc(ast_context: ^AstContext, symbol: ^Symbol, short_signature := true) {
 	if short_signature {
-		symbol.signature = get_short_signature(ast_context, symbol)
+		symbol.signature = get_short_signature(ast_context, symbol^)
 	} else {
-		symbol.signature = get_signature(ast_context, symbol)
+		symbol.signature = get_signature(ast_context, symbol^)
 	}
 
 	if symbol.doc == "" && symbol.comment == "" {
@@ -144,7 +144,7 @@ construct_symbol_docs :: proc(docs, comment: string, allocator := context.temp_a
 }
 
 // Returns the fully detailed signature for the symbol, including things like attributes and fields
-get_signature :: proc(ast_context: ^AstContext, symbol: ^Symbol) -> string {
+get_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string {
 	show_type_info := symbol.type == .Variable || symbol.type == .Field
 
 	pointer_prefix := repeat("^", symbol.pointers, ast_context.allocator)
@@ -236,6 +236,7 @@ get_signature :: proc(ast_context: ^AstContext, symbol: ^Symbol) -> string {
 		sb := strings.builder_make(ast_context.allocator)
 		if show_type_info {
 			append_type_information(&sb, ast_context, symbol, pointer_prefix)
+			strings.write_string(&sb, " :: ")
 		}
 		write_procedure_symbol_signature(&sb, v, detailed_signature=true)
 		return strings.to_string(sb)
@@ -284,7 +285,7 @@ get_signature :: proc(ast_context: ^AstContext, symbol: ^Symbol) -> string {
 	return get_short_signature(ast_context, symbol)
 }
 
-get_short_signature :: proc(ast_context: ^AstContext, symbol: ^Symbol) -> string {
+get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string {
 	// TODO: this is also a bit much, might need to clean it up into a function
 	show_type_info := (symbol.type == .Variable || symbol.type == .Field) && !(.Anonymous in symbol.flags) && symbol.type_name != ""
 
@@ -336,11 +337,12 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: ^Symbol) -> string
 		sb := strings.builder_make(ast_context.allocator)
 		if show_type_info {
 			append_type_information(&sb, ast_context, symbol, pointer_prefix)
+			strings.write_string(&sb, " :: ")
 		}
 		write_procedure_symbol_signature(&sb, v, detailed_signature=false)
 		return strings.to_string(sb)
 	case SymbolAggregateValue:
-		return "proc"
+		return "proc (..)"
 	case SymbolStructValue:
 		sb := strings.builder_make(ast_context.allocator)
 		if show_type_info {
@@ -437,10 +439,10 @@ get_bit_field_field_signature :: proc(value: SymbolBitFieldValue, index: int, al
 	return strings.to_string(sb)
 }
 
-write_symbol_type_information :: proc(ast_context: ^AstContext, sb: ^strings.Builder, symbol: ^Symbol, pointer_prefix: string) {
+write_symbol_type_information :: proc(ast_context: ^AstContext, sb: ^strings.Builder, symbol: Symbol, pointer_prefix: string) {
 	append_type_pkg := false
 	pkg_name := get_pkg_name(ast_context, symbol.type_pkg)
-	if pkg_name != "" {
+	if pkg_name != "" && pkg_name != "$builtin" {
 		if _, ok := keywords_docs[symbol.type_name]; !ok {
 			append_type_pkg = true
 		}
@@ -452,18 +454,7 @@ write_symbol_type_information :: proc(ast_context: ^AstContext, sb: ^strings.Bui
 	}
 }
 
-write_procedure_symbol_signature :: proc(sb: ^strings.Builder, value: SymbolProcedureValue, detailed_signature: bool) {
-	if detailed_signature {
-		if value.inlining == .Inline {
-			strings.write_string(sb, "#force_inline ")
-		} else if value.inlining == .No_Inline {
-			strings.write_string(sb, "#force_no_inline ")
-		}
-	}
-	strings.write_string(sb, "proc")
-	if s, ok := value.calling_convention.(string); ok && detailed_signature {
-		fmt.sbprintf(sb, " %s ", s)
-	}
+write_proc_param_list_and_return :: proc(sb: ^strings.Builder, value: SymbolProcedureValue) {
 	strings.write_string(sb, "(")
 	for arg, i in value.orig_arg_types {
 		build_string_node(arg, sb, false)
@@ -493,6 +484,22 @@ write_procedure_symbol_signature :: proc(sb: ^strings.Builder, value: SymbolProc
 	} else if value.diverging {
 		strings.write_string(sb, " -> !")
 	}
+
+}
+
+write_procedure_symbol_signature :: proc(sb: ^strings.Builder, value: SymbolProcedureValue, detailed_signature: bool) {
+	if detailed_signature {
+		if value.inlining == .Inline {
+			strings.write_string(sb, "#force_inline ")
+		} else if value.inlining == .No_Inline {
+			strings.write_string(sb, "#force_no_inline ")
+		}
+	}
+	strings.write_string(sb, "proc")
+	if s, ok := value.calling_convention.(string); ok && detailed_signature {
+		fmt.sbprintf(sb, " %s ", s)
+	}
+	write_proc_param_list_and_return(sb, value)
 	if detailed_signature {
 		for tag in value.tags {
 			s := ""
@@ -607,7 +614,7 @@ write_struct_hover :: proc(ast_context: ^AstContext, sb: ^strings.Builder, v: Sy
 append_type_information :: proc(
 	sb: ^strings.Builder,
 	ast_context: ^AstContext,
-	symbol: ^Symbol,
+	symbol: Symbol,
 	pointer_prefix: string,
 ) {
 	pkg_name := get_pkg_name(ast_context, symbol.type_pkg)
diff --git a/tests/completions_test.odin b/tests/completions_test.odin
index 9f01590..b704af3 100644
--- a/tests/completions_test.odin
+++ b/tests/completions_test.odin
@@ -258,7 +258,7 @@ ast_completion_identifier_proc_group :: proc(t: ^testing.T) {
 		packages = {},
 	}
 
-	test.expect_completion_docs(t, &source, "", {"test.group_function: proc"})
+	test.expect_completion_docs(t, &source, "", {"test.group_function: proc (..)"})
 }
 
 @(test)