Merge pull request #774 from BradLewis/feat/move-comments-to-docs

Moves comments to the end of the docs and uses markdown for completion documentation

8 files changed, 102 insertions, 96 deletions

diff --git a/src/server/analysis.odin b/src/server/analysis.odin
index ab1860d..2edae61 100644
--- a/src/server/analysis.odin
+++ b/src/server/analysis.odin
@@ -2740,7 +2740,7 @@ get_using_packages :: proc(ast_context: ^AstContext) -> []string {
 	return usings
 }
 
-get_symbol_pkg_name :: proc(ast_context: ^AstContext, symbol: Symbol) -> string {
+get_symbol_pkg_name :: proc(ast_context: ^AstContext, symbol: ^Symbol) -> string {
 	return get_pkg_name(ast_context, symbol.pkg)
 }
 
diff --git a/src/server/completion.odin b/src/server/completion.odin
index 054c9c7..78da5eb 100644
--- a/src/server/completion.odin
+++ b/src/server/completion.odin
@@ -157,6 +157,9 @@ get_completion_list :: proc(
 		get_package_completion(&ast_context, &position_context, &list)
 	}
 
+	// 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)
 	}
@@ -493,7 +496,7 @@ get_selector_completion :: proc(
 
 		for type in v.types {
 			if symbol, ok := resolve_type_expression(ast_context, type); ok {
-				base := get_symbol_pkg_name(ast_context, symbol)
+				base := get_symbol_pkg_name(ast_context, &symbol)
 
 				item := CompletionItem {
 					kind          = .EnumMember,
@@ -517,7 +520,7 @@ get_selector_completion :: proc(
 					item.label = fmt.aprintf(
 						"(%v%v.%v)",
 						repeat("^", symbol.pointers, context.temp_allocator),
-						get_symbol_pkg_name(ast_context, symbol),
+						get_symbol_pkg_name(ast_context, &symbol),
 						node_to_string(type, true),
 					)
 				}
@@ -607,7 +610,7 @@ get_selector_completion :: proc(
 				symbol.type = .Field
 				symbol.doc = get_doc(v.docs[i], context.temp_allocator)
 				symbol.comment = get_comment(v.comments[i])
-				symbol.signature = get_short_signature(ast_context, symbol)
+				build_documentation(ast_context, &symbol)
 
 				item := CompletionItem {
 					label         = name,
@@ -681,7 +684,7 @@ get_selector_completion :: proc(
 				}
 
 				resolve_unresolved_symbol(ast_context, &symbol)
-				symbol.signature = get_short_signature(ast_context, symbol)
+				build_documentation(ast_context, &symbol)
 
 				item := CompletionItem {
 					label         = symbol.name,
@@ -1322,7 +1325,8 @@ get_identifier_completion :: proc(
 		for r in results {
 			r := r
 			resolve_unresolved_symbol(ast_context, &r.symbol)
-			r.symbol.signature = get_short_signature(ast_context, r.symbol)
+
+			build_documentation(ast_context, &r.symbol)
 
 			uri, _ := common.parse_uri(r.symbol.uri, context.temp_allocator)
 			if uri.path != ast_context.fullpath {
@@ -1354,7 +1358,7 @@ get_identifier_completion :: proc(
 
 		if symbol, ok := resolve_type_identifier(ast_context, ident^); ok {
 			symbol.name = k
-			symbol.signature = get_short_signature(ast_context, symbol)
+			build_documentation(ast_context, &symbol)
 
 			if score, ok := common.fuzzy_match(matcher, ident.name); ok == 1 {
 				append(&combined, CombinedResult{score = score * 1.1, symbol = symbol})
@@ -1382,7 +1386,7 @@ get_identifier_completion :: proc(
 			ident.name = k
 
 			if symbol, ok := resolve_type_identifier(ast_context, ident^); ok {
-				symbol.signature = get_short_signature(ast_context, symbol)
+				build_documentation(ast_context, &symbol)
 
 				if score, ok := common.fuzzy_match(matcher, ident.name); ok == 1 {
 					symbol.name = clean_ident(ident.name)
@@ -1665,7 +1669,7 @@ get_type_switch_completion :: proc(
 						item.label = fmt.aprintf(
 							"%v%v.%v",
 							repeat("^", symbol.pointers, context.temp_allocator),
-							get_symbol_pkg_name(ast_context, symbol),
+							get_symbol_pkg_name(ast_context, &symbol),
 							name,
 						)
 						item.detail = item.label
@@ -2125,18 +2129,9 @@ format_to_label_details :: proc(list: ^CompletionList) {
 		// log.errorf("item:%v: %v:%v", item.kind, item.label, item.detail)
 		#partial switch item.kind {
 		case .Function:
-			comment := ""
 			proc_info := ""
-			detail_split := strings.split_n(item.detail, "\n", 2)
-			if len(detail_split) == 1 {
-				// We have no comment
-				proc_info = detail_split[0]
-			} else if len(detail_split) == 2 {
-				comment = detail_split[0]
-				proc_info = detail_split[1]
-			}
 			// Split the leading name of the proc
-			proc_info_split := strings.split_n(proc_info, " proc", 2)
+			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 = "(..)"
@@ -2146,20 +2141,13 @@ format_to_label_details :: proc(list: ^CompletionList) {
 
 			item.labelDetails = CompletionItemLabelDetails {
 				detail      = proc_info,
-				description = fmt.tprintf(" %s", comment),
+				description = "",
 			}
 		case .Variable, .Constant, .Field:
 			type_index := strings.index(item.detail, ":")
-			type_name := item.detail[type_index + 1:]
-
-			commentIndex := strings.index(type_name, "/")
-			if commentIndex > 0 {
-				type_name, _ = strings.substring(type_name, 0, commentIndex)
-			}
-
 			item.labelDetails = CompletionItemLabelDetails {
 				detail      = "",
-				description = type_name,
+				description = item.detail[type_index + 1:],
 			}
 		case .Struct, .Enum, .Class:
 			type_index := strings.index(item.detail, ":")
@@ -2183,6 +2171,17 @@ format_to_label_details :: proc(list: ^CompletionList) {
 	}
 }
 
+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 417815a..cd22b90 100644
--- a/src/server/documentation.odin
+++ b/src/server/documentation.odin
@@ -104,7 +104,47 @@ keywords_docs: map[string]bool = {
 	"where"         = true,
 }
 
-get_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string {
+// 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)
+	} else {
+		symbol.signature = get_signature(ast_context, symbol)
+	}
+
+	if symbol.doc == "" && symbol.comment == "" {
+		return
+	}
+
+	symbol.doc = construct_symbol_docs(symbol.doc, symbol.comment)
+}
+
+// Adds signature and docs information for a bit field field
+build_bit_field_field_documentation :: proc(
+	ast_context: ^AstContext, symbol: ^Symbol, value: SymbolBitFieldValue, index: int, allocator := context.temp_allocator
+) {
+	symbol.signature = get_bit_field_field_signature(value, index, allocator)
+	symbol.doc = construct_symbol_docs(symbol.doc, symbol.comment)
+}
+
+construct_symbol_docs :: proc(docs, comment: string, allocator := context.temp_allocator) -> string {
+	sb := strings.builder_make(allocator = context.temp_allocator)
+	if docs != "" {
+		strings.write_string(&sb, docs)
+		if comment != "" {
+			strings.write_string(&sb, "\n")
+		}
+	}
+
+	if comment != "" {
+	    fmt.sbprintf(&sb, "\n```odin\n%s\n```", comment)
+	}
+
+	 return strings.to_string(sb)
+}
+
+// Returns the fully detailed signature for the symbol, including things like attributes and fields
+get_signature :: proc(ast_context: ^AstContext, symbol: ^Symbol) -> string {
 	is_variable := symbol.type == .Variable
 
 	pointer_prefix := repeat("^", symbol.pointers, ast_context.allocator)
@@ -251,7 +291,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 {
 	is_variable := symbol.type == .Variable
 
 	pointer_prefix := repeat("^", symbol.pointers, ast_context.allocator)
@@ -271,18 +311,12 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string
 			strings.write_string(&sb, pointer_prefix)
 			build_string_node(v.ident, &sb, false)
 		}
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolBitSetValue:
 		sb := strings.builder_make(ast_context.allocator)
 		fmt.sbprintf(&sb, "%sbit_set[", pointer_prefix)
 		build_string_node(v.expr, &sb, false)
 		strings.write_string(&sb, "]")
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolEnumValue:
 		sb := strings.builder_make(ast_context.allocator)
@@ -294,9 +328,6 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string
 			strings.write_string(&sb, pointer_prefix)
 			strings.write_string(&sb, "enum {..}")
 		}
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolMapValue:
 		sb := strings.builder_make(ast_context.allocator)
@@ -304,9 +335,6 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string
 		build_string_node(v.key, &sb, false)
 		strings.write_string(&sb, "]")
 		build_string_node(v.value, &sb, false)
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolProcedureValue:
 		sb := strings.builder_make(ast_context.allocator)
@@ -328,9 +356,6 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string
 			strings.write_string(&sb, pointer_prefix)
 			strings.write_string(&sb, "struct {..}")
 		}
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolUnionValue:
 		sb := strings.builder_make(ast_context.allocator)
@@ -342,9 +367,6 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string
 			strings.write_string(&sb, pointer_prefix)
 			strings.write_string(&sb, "union {..}")
 		}
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolBitFieldValue:
 		sb := strings.builder_make(ast_context.allocator)
@@ -357,34 +379,22 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string
 			build_string_node(v.backing_type, &sb, false)
 			strings.write_string(&sb, " {..}")
 		}
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 		
 	case SymbolMultiPointerValue:
 		sb := strings.builder_make(ast_context.allocator)
 		fmt.sbprintf(&sb, "%s[^]", pointer_prefix)
 		build_string_node(v.expr, &sb, false)
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolDynamicArrayValue:
 		sb := strings.builder_make(ast_context.allocator)
 		fmt.sbprintf(&sb, "%s[dynamic]", pointer_prefix)
 		build_string_node(v.expr, &sb, false)
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolSliceValue:
 		sb := strings.builder_make(ast_context.allocator)
 		fmt.sbprintf(&sb, "%s[]", pointer_prefix)
 		build_string_node(v.expr, &sb, false)
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolFixedArrayValue:
 		sb := strings.builder_make(ast_context.allocator)
@@ -392,9 +402,6 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string
 		build_string_node(v.len, &sb, false)
 		strings.write_string(&sb, "]")
 		build_string_node(v.expr, &sb, false)
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolMatrixValue:
 		sb := strings.builder_make(ast_context.allocator)
@@ -404,9 +411,6 @@ get_short_signature :: proc(ast_context: ^AstContext, symbol: Symbol) -> string
 		build_string_node(v.y, &sb, false)
 		strings.write_string(&sb, "]")
 		build_string_node(v.expr, &sb, false)
-		if symbol.comment != "" {
-			fmt.sbprintf(&sb, " %s", symbol.comment)
-		}
 		return strings.to_string(sb)
 	case SymbolPackageValue:
 		return "package"
@@ -441,11 +445,10 @@ get_bit_field_field_signature :: proc(value: SymbolBitFieldValue, index: int, al
 	build_string_node(value.types[index], &sb, false)
 	strings.write_string(&sb, " | ")
 	build_string_node(value.bit_sizes[index], &sb, false)
-	append_comments(&sb, value.comments, index)
 	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 != "" {
@@ -588,7 +591,7 @@ write_struct_hover :: proc(ast_context: ^AstContext, sb: ^strings.Builder, v: Sy
 append_variable_full_name :: proc(
 	sb: ^strings.Builder,
 	ast_context: ^AstContext,
-	symbol: Symbol,
+	symbol: ^Symbol,
 	pointer_prefix: string,
 ) {
 	pkg_name := get_symbol_pkg_name(ast_context, symbol)
@@ -644,17 +647,11 @@ concatenate_raw_string_information :: proc(
 
 	if type == .Package {
 		return fmt.tprintf("%v: package", name)
-	//} else if type == .Keyword {
-	//	return name
-	} else {
-		sb := strings.builder_make()
-		if (type == .Function || type == .Type_Function) && comment != "" {
-			fmt.sbprintf(&sb, "%s\n", comment)
-		}
-		fmt.sbprintf(&sb, "%v.%v", pkg, name)
-		if signature != "" {
-			fmt.sbprintf(&sb, ": %v", signature)
-		}
-		return strings.to_string(sb)
 	}
+	sb := strings.builder_make()
+	fmt.sbprintf(&sb, "%v.%v", pkg, name)
+	if signature != "" {
+		fmt.sbprintf(&sb, ": %v", signature)
+	}
+	return strings.to_string(sb)
 }
diff --git a/src/server/hover.odin b/src/server/hover.odin
index f23bc23..919a041 100644
--- a/src/server/hover.odin
+++ b/src/server/hover.odin
@@ -174,7 +174,7 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 										symbol.comment = get_comment(value.comments[field_index + name_index])
 										symbol.doc = get_doc(value.docs[field_index + name_index], context.temp_allocator)
 
-										symbol.signature = get_short_signature(&ast_context, symbol)
+										build_documentation(&ast_context, &symbol, true)
 										hover.contents = write_hover_content(&ast_context, symbol)
 										return hover, true, true
 									}
@@ -203,7 +203,8 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 									symbol.name = identifier.name
 									symbol.comment = get_comment(value.comments[i])
 									symbol.doc = get_doc(value.docs[i], context.temp_allocator)
-									symbol.signature = get_bit_field_field_signature(value, i)
+
+									build_bit_field_field_documentation(&ast_context, &symbol, value, i)
 									hover.contents = write_hover_content(&ast_context, symbol)
 									return hover, true, true
 								}
@@ -252,7 +253,7 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 
 		if position_context.call != nil {
 			if symbol, ok := resolve_type_location_proc_param_name(&ast_context, &position_context); ok {
-				symbol.signature = get_signature(&ast_context, symbol)
+				build_documentation(&ast_context, &symbol, false)
 				hover.contents = write_hover_content(&ast_context, symbol)
 				return hover, true, true
 			}
@@ -274,7 +275,7 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 
 			if position_in_node(base, position_context.position) {
 				if resolved, ok := resolve_type_identifier(&ast_context, ident); ok {
-					resolved.signature = get_signature(&ast_context, resolved)
+					build_documentation(&ast_context, &resolved, false)
 					resolved.name = ident.name
 
 					if resolved.type == .Variable {
@@ -329,7 +330,7 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 						symbol.pkg = selector.name
 						symbol.comment = get_comment(v.comments[i])
 						symbol.doc = get_doc(v.docs[i], context.temp_allocator)
-						symbol.signature = get_short_signature(&ast_context, symbol)
+						build_documentation(&ast_context, &symbol, true)
 						hover.contents = write_hover_content(&ast_context, symbol)
 						return hover, true, true
 					}
@@ -341,8 +342,10 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 					if symbol, ok := resolve_type_expression(&ast_context, v.types[i]); ok {
 						symbol.name = name
 						symbol.pkg = selector.name
-						symbol.signature = get_bit_field_field_signature(v, i)
 						symbol.doc = get_doc(v.docs[i], ast_context.allocator)
+						symbol.comment = get_comment(v.comments[i])
+
+						build_bit_field_field_documentation(&ast_context, &symbol, v, i)
 						hover.contents = write_hover_content(&ast_context, symbol)
 						return hover, true, true
 					}
@@ -360,7 +363,7 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 						}
 					}
 					if resolved, ok := resolve_type_identifier(&ast_context, ident^); ok {
-						resolved.signature = get_signature(&ast_context, resolved)
+						build_documentation(&ast_context, &resolved, false)
 						resolved.name = ident.name
 
 						if resolved.type == .Variable {
@@ -448,7 +451,7 @@ get_hover_information :: proc(document: ^Document, position: common.Position) ->
 		}
 
 		if resolved, ok := resolve_type_identifier(&ast_context, ident); ok {
-			resolved.signature = get_signature(&ast_context, resolved)
+			build_documentation(&ast_context, &resolved, false)
 			resolved.name = ident.name
 
 			if resolved.type == .Variable {
diff --git a/src/server/methods.odin b/src/server/methods.odin
index f17be51..4b8e74e 100644
--- a/src/server/methods.odin
+++ b/src/server/methods.odin
@@ -67,7 +67,7 @@ append_method_completion :: proc(
 		if symbols, ok := &v.methods[method]; ok {
 			for &symbol in symbols {
 				resolve_unresolved_symbol(ast_context, &symbol)
-				symbol.signature = get_short_signature(ast_context, symbol)
+				build_documentation(ast_context, &symbol)
 
 				range, ok := get_range_from_selection_start_to_dot(position_context)
 
@@ -113,7 +113,7 @@ append_method_completion :: proc(
 				if symbol.pkg != ast_context.document_package {
 					new_text = fmt.tprintf(
 						"%v.%v",
-						path.base(get_symbol_pkg_name(ast_context, symbol), false, ast_context.allocator),
+						path.base(get_symbol_pkg_name(ast_context, &symbol), false, ast_context.allocator),
 						symbol.name,
 					)
 				} else {
@@ -125,11 +125,12 @@ append_method_completion :: proc(
 				} else {
 					new_text = fmt.tprintf("%v(%v%v%v)$0", new_text, references, receiver, dereferences)
 				}
+				build_documentation(ast_context, &symbol)
 
 				item := CompletionItem {
 					label = symbol.name,
 					kind = symbol_type_to_completion_kind(symbol.type),
-					detail = get_short_signature(ast_context, symbol),
+					detail = symbol.signature,
 					additionalTextEdits = remove_edit,
 					textEdit = TextEdit{newText = new_text, range = {start = range.end, end = range.end}},
 					insertTextFormat = .Snippet,
diff --git a/src/server/signature.odin b/src/server/signature.odin
index 00e6fb9..03634d5 100644
--- a/src/server/signature.odin
+++ b/src/server/signature.odin
@@ -134,7 +134,7 @@ get_signature_information :: proc(document: ^Document, position: common.Position
 			parameters[i].label = node_to_string(arg)
 		}
 
-		call.signature = get_short_signature(&ast_context, call)
+		build_documentation(&ast_context, &call)
 
 		info := SignatureInformation {
 			label         = concatenate_symbol_information(&ast_context, call),
@@ -160,7 +160,7 @@ get_signature_information :: proc(document: ^Document, position: common.Position
 					parameters[i].label = node_to_string(arg)
 				}
 
-				symbol.signature = get_short_signature(&ast_context, symbol)
+				build_documentation(&ast_context, &symbol)
 
 				info := SignatureInformation {
 					label         = concatenate_symbol_information(&ast_context, symbol),
diff --git a/src/server/types.odin b/src/server/types.odin
index 1da5f3c..78a7158 100644
--- a/src/server/types.odin
+++ b/src/server/types.odin
@@ -355,11 +355,16 @@ InsertTextMode :: enum {
 	adjustIndentation = 2,
 }
 
+CompletionDocumention :: union {
+	MarkupContent,
+	string,
+}
+
 CompletionItem :: struct {
 	label:               string,
 	kind:                CompletionItemKind,
 	detail:              string,
-	documentation:       string,
+	documentation:       CompletionDocumention,
 	insertTextFormat:    Maybe(InsertTextFormat),
 	insertText:          Maybe(string),
 	InsertTextMode:      Maybe(InsertTextMode),
diff --git a/src/testing/testing.odin b/src/testing/testing.odin
index d80b727..429d393 100644
--- a/src/testing/testing.odin
+++ b/src/testing/testing.odin
@@ -265,7 +265,8 @@ expect_hover :: proc(t: ^testing.T, src: ^Source, expect_hover_string: string) {
 		return
 	}
 
-	content_without_markdown, _ := strings.remove(hover.contents.value[8:], "\n```", 1, context.temp_allocator)
+	first_strip, _ := strings.remove(hover.contents.value, "```odin\n", 2, context.temp_allocator)
+	content_without_markdown, _ := strings.remove(first_strip, "\n```", 2, context.temp_allocator)
 
 	if content_without_markdown != expect_hover_string {
 		log.errorf("Expected hover string:\n%q, but received:\n%q", expect_hover_string, content_without_markdown)