Update `odin doc` to support multiple package outputs by passing multiple paths; Replace `-all` with `-short`

Example:
odin doc core/path core/path/filepath

5 files changed, 171 insertions, 109 deletions

diff --git a/src/build_settings.cpp b/src/build_settings.cpp
index 35dc9a7c1..c251cba53 100644
--- a/src/build_settings.cpp
+++ b/src/build_settings.cpp
@@ -130,7 +130,7 @@ char const *odin_command_strings[32] = {
 
 
 enum CmdDocFlag : u32 {
-	CmdDocFlag_All = 1<<0,
+	CmdDocFlag_Short       = 1<<0,
 	CmdDocFlag_AllPackages = 1<<1,
 };
 
@@ -200,7 +200,7 @@ struct BuildContext {
 	bool   linker_map_file;
 
 	u32 cmd_doc_flags;
-	Array<String> doc_packages;
+	Array<String> extra_packages;
 
 	QueryDataSetSettings query_data_set_settings;
 
diff --git a/src/docs.cpp b/src/docs.cpp
index d86f85c8d..aa1b89560 100644
--- a/src/docs.cpp
+++ b/src/docs.cpp
@@ -93,6 +93,8 @@ bool print_doc_comment_group_string(i32 indent, CommentGroup *g) {
 	isize count = 0;
 	for_array(i, g->list) {
 		String comment = g->list[i].string;
+		String original_comment = comment;
+
 		bool slash_slash = comment[1] == '/';
 		bool slash_star = comment[1] == '*';
 		if (comment[1] == '/') {
@@ -113,15 +115,51 @@ bool print_doc_comment_group_string(i32 indent, CommentGroup *g) {
 			if (string_starts_with(comment, str_lit("+"))) {
 				continue;
 			}
+			if (string_starts_with(comment, str_lit("@("))) {
+				continue;
+			}
 		}
-		if (string_starts_with(comment, str_lit("@("))) {
-			continue;
+
+		if (slash_slash) {
+			print_doc_line(indent, "%.*s", LIT(comment));
+			count += 1;
+		} else {
+			isize pos = 0;
+			for (; pos < comment.len; pos++) {
+				isize end = pos;
+				for (; end < comment.len; end++) {
+					if (comment[end] == '\n') {
+						break;
+					}
+				}
+				String line = substring(comment, pos, end);
+				pos = end+1;
+				String trimmed_line = string_trim_whitespace(line);
+				if (trimmed_line.len == 0) {
+					if (count == 0) {
+						continue;
+					}
+				}
+				/*
+				 * Remove comments with
+				 * styles
+				 * like this
+				 */
+				if (string_starts_with(line, str_lit("* "))) {
+					line = substring(line, 2, line.len);
+				}
+
+				print_doc_line(indent, "%.*s", LIT(line));
+				count += 1;
+			}
 		}
+	}
 
-		print_doc_line(indent, "%.*s", LIT(comment));
-		count += 1;
+	if (count > 0) {
+		print_doc_line(0, "");
+		return true;
 	}
-	return count > 0;
+	return false;
 }
 
 
@@ -129,10 +167,10 @@ bool print_doc_comment_group_string(i32 indent, CommentGroup *g) {
 
 void print_doc_expr(Ast *expr) {
 	gbString s = nullptr;
-	if (build_context.cmd_doc_flags & CmdDocFlag_All) {
-		s = expr_to_string(expr);
-	} else {
+	if (build_context.cmd_doc_flags & CmdDocFlag_Short) {
 		s = expr_to_string_shorthand(expr);
+	} else {
+		s = expr_to_string(expr);
 	}
 	gb_file_write(gb_file_get_standard(gbFileStandard_Output), s, gb_string_length(s));
 	gb_string_free(s);
@@ -151,9 +189,7 @@ void print_doc_package(CheckerInfo *info, AstPackage *pkg) {
 		AstFile *f = pkg->files[i];
 		if (f->pkg_decl) {
 			GB_ASSERT(f->pkg_decl->kind == Ast_PackageDecl);
-			if (print_doc_comment_group_string(1, f->pkg_decl->PackageDecl.docs)) {
-				print_doc_line(0, "");
-			}
+			print_doc_comment_group_string(1, f->pkg_decl->PackageDecl.docs);
 		}
 	}
 
@@ -182,6 +218,8 @@ void print_doc_package(CheckerInfo *info, AstPackage *pkg) {
 		}
 		gb_sort_array(entities.data, entities.count, cmp_entities_for_printing);
 
+		bool show_docs = (build_context.cmd_doc_flags & CmdDocFlag_Short) == 0;
+
 		EntityKind curr_entity_kind = Entity_Invalid;
 		for_array(i, entities) {
 			Entity *e = entities[i];
@@ -192,6 +230,7 @@ void print_doc_package(CheckerInfo *info, AstPackage *pkg) {
 				continue;
 			}
 
+
 			if (curr_entity_kind != e->kind) {
 				if (curr_entity_kind != Entity_Invalid) {
 					print_doc_line(0, "");
@@ -213,6 +252,7 @@ void print_doc_package(CheckerInfo *info, AstPackage *pkg) {
 				docs = e->decl_info->docs;
 			}
 			GB_ASSERT(type_expr != nullptr || init_expr != nullptr);
+
 			print_doc_line_no_newline(2, "%.*s", LIT(e->token.string));
 			if (type_expr != nullptr) {
 				gbString t = expr_to_string(type_expr);
@@ -233,14 +273,8 @@ void print_doc_package(CheckerInfo *info, AstPackage *pkg) {
 
 			gb_printf(";\n");
 
-
-			if (build_context.cmd_doc_flags & CmdDocFlag_All) {
-				if (comment) {
-					// gb_printf(" <comment>");
-				}
-				if (print_doc_comment_group_string(3, docs)) {
-					gb_printf("\n");
-				}
+			if (show_docs) {
+				print_doc_comment_group_string(3, docs);
 			}
 		}
 		print_doc_line(0, "");
@@ -248,7 +282,8 @@ void print_doc_package(CheckerInfo *info, AstPackage *pkg) {
 
 	if (pkg->fullpath.len != 0) {
 		print_doc_line(0, "");
-		print_doc_line(1, "fullpath: %.*s", LIT(pkg->fullpath));
+		print_doc_line(1, "fullpath:");
+		print_doc_line(2, "%.*s", LIT(pkg->fullpath));
 		print_doc_line(1, "files:");
 		for_array(i, pkg->files) {
 			AstFile *f = pkg->files[i];
@@ -262,51 +297,23 @@ void print_doc_package(CheckerInfo *info, AstPackage *pkg) {
 void generate_documentation(Checker *c) {
 	CheckerInfo *info = &c->info;
 
-	if (build_context.doc_packages.count != 0) {
-		auto pkgs = array_make<AstPackage *>(permanent_allocator(), 0, info->packages.entries.count);
-		bool was_error = false;
-		for_array(j, build_context.doc_packages) {
-			bool found = false;
-			String name = build_context.doc_packages[j];
-			for_array(i, info->packages.entries) {
-				AstPackage *pkg = info->packages.entries[i].value;
-				if (name == pkg->name) {
-					found = true;
-					array_add(&pkgs, pkg);
-					break;
-				}
-			}
-			if (!found) {
-				gb_printf_err("Unknown package %.*s\n", LIT(name));
-				was_error = true;
-			}
-		}
-		if (was_error) {
-			gb_exit(1);
-			return;
-		}
-
-		gb_sort_array(pkgs.data, pkgs.count, cmp_ast_package_by_name);
-
-		for_array(i, pkgs) {
-			print_doc_package(info, pkgs[i]);
-		}
-	} else if (build_context.cmd_doc_flags & CmdDocFlag_AllPackages) {
-		auto pkgs = array_make<AstPackage *>(permanent_allocator(), 0, info->packages.entries.count);
-		for_array(i, info->packages.entries) {
-			AstPackage *pkg = info->packages.entries[i].value;
+	auto pkgs = array_make<AstPackage *>(permanent_allocator(), 0, info->packages.entries.count);
+	for_array(i, info->packages.entries) {
+		AstPackage *pkg = info->packages.entries[i].value;
+		if (build_context.cmd_doc_flags & CmdDocFlag_AllPackages) {
 			array_add(&pkgs, pkg);
+		} else {
+			if (pkg->kind == Package_Init) {
+				array_add(&pkgs, pkg);
+			} else if (pkg->is_extra) {
+				array_add(&pkgs, pkg);
+			}
 		}
+	}
 
-		gb_sort_array(pkgs.data, pkgs.count, cmp_ast_package_by_name);
-
-		for_array(i, pkgs) {
-			print_doc_package(info, pkgs[i]);
-		}
-	} else {
-		GB_ASSERT(info->init_scope->flags & ScopeFlag_Pkg);
-		AstPackage *pkg = info->init_scope->pkg;
-		print_doc_package(info, pkg);
+	gb_sort_array(pkgs.data, pkgs.count, cmp_ast_package_by_name);
 
+	for_array(i, pkgs) {
+		print_doc_package(info, pkgs[i]);
 	}
 }
diff --git a/src/main.cpp b/src/main.cpp
index 7eef5d2ad..dbade085d 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -589,8 +589,7 @@ enum BuildFlagKind {
 	BuildFlag_GlobalDefinitions,
 	BuildFlag_GoToDefinitions,
 
-	BuildFlag_Package,
-	BuildFlag_All,
+	BuildFlag_Short,
 	BuildFlag_AllPackages,
 
 #if defined(GB_SYSTEM_WINDOWS)
@@ -699,9 +698,8 @@ bool parse_build_flags(Array<String> args) {
 	add_flag(&build_flags, BuildFlag_GlobalDefinitions, str_lit("global-definitions"), BuildFlagParam_None, Command_query);
 	add_flag(&build_flags, BuildFlag_GoToDefinitions,   str_lit("go-to-definitions"),  BuildFlagParam_None, Command_query);
 
-	add_flag(&build_flags, BuildFlag_Package,     str_lit("package"),       BuildFlagParam_String, Command_doc, true);
-	add_flag(&build_flags, BuildFlag_All,         str_lit("all"),           BuildFlagParam_None,   Command_doc);
-	add_flag(&build_flags, BuildFlag_AllPackages, str_lit("all-packages"),  BuildFlagParam_None,   Command_doc);
+	add_flag(&build_flags, BuildFlag_Short,         str_lit("short"),        BuildFlagParam_None, Command_doc);
+	add_flag(&build_flags, BuildFlag_AllPackages,   str_lit("all-packages"), BuildFlagParam_None, Command_doc);
 
 
 #if defined(GB_SYSTEM_WINDOWS)
@@ -852,7 +850,7 @@ bool parse_build_flags(Array<String> args) {
 							GB_ASSERT(value.kind == ExactValue_String);
 							String path = value.value_string;
 							path = string_trim_whitespace(path);
-							if (is_import_path_valid(path)) {
+							if (is_build_flag_path_valid(path)) {
 								#if defined(GB_SYSTEM_WINDOWS)
 									String ext = path_extension(path);
 									if (ext == ".exe") {
@@ -1191,22 +1189,15 @@ bool parse_build_flags(Array<String> args) {
 							}
 							break;
 
-						case BuildFlag_Package:
-							GB_ASSERT(value.kind == ExactValue_String);
-							if (value.value_string.len == 0) {
-								gb_printf_err("Invalid use of -package flag\n");
-							} else {
-								array_add(&build_context.doc_packages, value.value_string);
-							}
-							break;
-						case BuildFlag_All:
-							build_context.cmd_doc_flags |= CmdDocFlag_All;
+						case BuildFlag_Short:
+							build_context.cmd_doc_flags |= CmdDocFlag_Short;
 							break;
 						case BuildFlag_AllPackages:
 							build_context.cmd_doc_flags |= CmdDocFlag_AllPackages;
 							break;
 
 
+
 					#if defined(GB_SYSTEM_WINDOWS)
 						case BuildFlag_IgnoreVsSearch:
 							GB_ASSERT(value.kind == ExactValue_Invalid);
@@ -1217,7 +1208,7 @@ bool parse_build_flags(Array<String> args) {
 							GB_ASSERT(value.kind == ExactValue_String);
 							String path = value.value_string;
 							path = string_trim_whitespace(path);
-							if (is_import_path_valid(path)) {
+							if (is_build_flag_path_valid(path)) {
 								if(!string_ends_with(path, str_lit(".rc"))) {
 									gb_printf_err("Invalid -resource path %.*s, missing .rc\n", LIT(path));
 									bad_flags = true;
@@ -1235,7 +1226,7 @@ bool parse_build_flags(Array<String> args) {
 							GB_ASSERT(value.kind == ExactValue_String);
 							String path = value.value_string;
 							path = string_trim_whitespace(path);
-							if (is_import_path_valid(path)) {
+							if (is_build_flag_path_valid(path)) {
 								// #if defined(GB_SYSTEM_WINDOWS)
 								// 	String ext = path_extension(path);
 								// 	if (ext != ".pdb") {
@@ -1299,12 +1290,6 @@ bool parse_build_flags(Array<String> args) {
 		}
 	}
 
-	if (build_context.doc_packages.count > 0 && set_flags[BuildFlag_AllPackages]) {
-		gb_printf_err("'odin doc' does not allow both flags together '-all-packages' and '-package' together");;
-		bad_flags = true;
-	}
-
-
 	if (build_context.query_data_set_settings.ok) {
 		if (build_context.query_data_set_settings.kind == QueryDataSet_Invalid) {
 			gb_printf_err("'odin query' requires a flag determining the kind of query data set to be returned\n");
@@ -1537,6 +1522,9 @@ void print_show_help(String const arg0, String const &command) {
 		print_usage_line(1, "query     [experimental] parse, type check, and output a .json file containing information about the program");
 	} else if (command == "doc") {
 		print_usage_line(1, "doc       generate documentation from a .odin file, or directory of .odin files");
+		print_usage_line(2, "Examples:");
+		print_usage_line(3, "odin doc core/path");
+		print_usage_line(3, "odin doc core/path core/path/filepath");
 	} else if (command == "version") {
 		print_usage_line(1, "version   print version");
 	}
@@ -1552,14 +1540,8 @@ void print_show_help(String const arg0, String const &command) {
 	print_usage_line(0, "");
 
 	if (doc) {
-		print_usage_line(1, "-all");
-		print_usage_line(2, "Show all documentation for the packages");
-		print_usage_line(0, "");
-
-		print_usage_line(1, "-package:<string>");
-		print_usage_line(2, "Add package name to generate documentation for");
-		print_usage_line(2, "Multiple flags are allowed");
-		print_usage_line(2, "Example: -doc:runtime");
+		print_usage_line(1, "-short");
+		print_usage_line(2, "Show shortened documentation for the packages");
 		print_usage_line(0, "");
 
 		print_usage_line(1, "-all-packages");
@@ -1833,7 +1815,7 @@ int main(int arg_count, char const **arg_ptr) {
 	add_library_collection(str_lit("core"), get_fullpath_relative(heap_allocator(), odin_root_dir(), str_lit("core")));
 
 	map_init(&build_context.defined_values, heap_allocator());
-	build_context.doc_packages.allocator = heap_allocator();
+	build_context.extra_packages.allocator = heap_allocator();
 
 
 	Array<String> args = setup_args(arg_count, arg_ptr);
@@ -1904,6 +1886,20 @@ int main(int arg_count, char const **arg_ptr) {
 
 		build_context.command_kind = Command_doc;
 		init_filename = args[2];
+		for (isize i = 3; i < args.count; i++) {
+			auto arg = args[i];
+			if (string_starts_with(arg, str_lit("-"))) {
+				break;
+			}
+			array_add(&build_context.extra_packages, arg);
+		}
+		isize extra_count = build_context.extra_packages.count;
+		if (extra_count > 0) {
+			gb_memmove(args.data + 3, args.data + 3 + extra_count, extra_count * gb_size_of(*args.data));
+			args.count -= extra_count;
+		}
+
+
 		build_context.no_output_files = true;
 		build_context.generate_docs = true;
 		build_context.no_entry_point = true; // ignore entry point
@@ -2005,8 +2001,11 @@ int main(int arg_count, char const **arg_ptr) {
 	temp_allocator_free_all(&temporary_allocator_data);
 
 	if (build_context.generate_docs) {
+		if (global_error_collector.count != 0) {
+			return 1;
+		}
 		generate_documentation(&checker);
-		return global_error_collector.count ? 1 : 0;
+		return 0;
 	}
 
 	if (build_context.no_output_files) {
diff --git a/src/parser.cpp b/src/parser.cpp
index 9e9708f9c..ec38dca9b 100644
--- a/src/parser.cpp
+++ b/src/parser.cpp
@@ -4653,14 +4653,14 @@ void parser_add_foreign_file_to_process(Parser *p, AstPackage *pkg, AstForeignFi
 
 
 // NOTE(bill): Returns true if it's added
-bool try_add_import_path(Parser *p, String const &path, String const &rel_path, TokenPos pos, PackageKind kind = Package_Normal) {
+AstPackage *try_add_import_path(Parser *p, String const &path, String const &rel_path, TokenPos pos, PackageKind kind = Package_Normal) {
 	String const FILE_EXT = str_lit(".odin");
 
 	gb_mutex_lock(&p->file_add_mutex);
 	defer (gb_mutex_unlock(&p->file_add_mutex));
 
 	if (string_set_exists(&p->imported_files, path)) {
-		return false;
+		return nullptr;
 	}
 	string_set_add(&p->imported_files, path);
 
@@ -4683,7 +4683,7 @@ bool try_add_import_path(Parser *p, String const &path, String const &rel_path,
 		pkg->is_single_file = true;
 		parser_add_file_to_process(p, pkg, fi, pos);
 		parser_add_package(p, pkg);
-		return true;
+		return pkg;
 	}
 
 
@@ -4699,22 +4699,22 @@ bool try_add_import_path(Parser *p, String const &path, String const &rel_path,
 	switch (rd_err) {
 	case ReadDirectory_InvalidPath:
 		syntax_error(pos, "Invalid path: %.*s", LIT(rel_path));
-		return false;
+		return nullptr;
 	case ReadDirectory_NotExists:
 		syntax_error(pos, "Path does not exist: %.*s", LIT(rel_path));
-		return false;
+		return nullptr;
 	case ReadDirectory_Permission:
 		syntax_error(pos, "Unknown error whilst reading path %.*s", LIT(rel_path));
-		return false;
+		return nullptr;
 	case ReadDirectory_NotDir:
 		syntax_error(pos, "Expected a directory for a package, got a file: %.*s", LIT(rel_path));
-		return false;
+		return nullptr;
 	case ReadDirectory_Empty:
 		syntax_error(pos, "Empty directory: %.*s", LIT(rel_path));
-		return false;
+		return nullptr;
 	case ReadDirectory_Unknown:
 		syntax_error(pos, "Unknown error whilst reading path %.*s", LIT(rel_path));
-		return false;
+		return nullptr;
 	}
 
 	for_array(list_index, list) {
@@ -4736,7 +4736,7 @@ bool try_add_import_path(Parser *p, String const &path, String const &rel_path,
 
 	parser_add_package(p, pkg);
 
-	return true;
+	return pkg;
 }
 
 gb_global Rune illegal_import_runes[] = {
@@ -4755,7 +4755,7 @@ bool is_import_path_valid(String path) {
 		u8 *curr = start;
 		while (curr < end) {
 			isize width = 1;
-			Rune r = curr[0];
+			Rune r = *curr;
 			if (r >= 0x80) {
 				width = gb_utf8_decode(curr, end-curr, &r);
 				if (r == GB_RUNE_INVALID && width == 1) {
@@ -4780,6 +4780,45 @@ bool is_import_path_valid(String path) {
 	return false;
 }
 
+bool is_build_flag_path_valid(String path) {
+	if (path.len > 0) {
+		u8 *start = path.text;
+		u8 *end = path.text + path.len;
+		u8 *curr = start;
+		isize index = 0;
+		while (curr < end) {
+			isize width = 1;
+			Rune r = *curr;
+			if (r >= 0x80) {
+				width = gb_utf8_decode(curr, end-curr, &r);
+				if (r == GB_RUNE_INVALID && width == 1) {
+					return false;
+				}
+				else if (r == GB_RUNE_BOM && curr-start > 0) {
+					return false;
+				}
+			}
+
+			for (isize i = 0; i < gb_count_of(illegal_import_runes); i++) {
+#if defined(GB_SYSTEM_WINDOWS)
+				if (r == '\\') {
+					break;
+				}
+#endif
+				if (r == illegal_import_runes[i]) {
+					return false;
+				}
+			}
+
+			curr += width;
+			index += 1;
+		}
+
+		return true;
+	}
+	return false;
+}
+
 
 bool is_package_name_reserved(String const &name) {
 	if (name == "builtin") {
@@ -5263,6 +5302,22 @@ ParseFileError parse_packages(Parser *p, String init_filename) {
 	try_add_import_path(p, init_fullpath, init_fullpath, init_pos, Package_Init);
 	p->init_fullpath = init_fullpath;
 
+	for_array(i, build_context.extra_packages) {
+		String path = build_context.extra_packages[i];
+		String fullpath = path_to_full_path(heap_allocator(), path); // LEAK?
+		if (!path_is_directory(fullpath)) {
+			String const ext = str_lit(".odin");
+			if (!string_ends_with(fullpath, ext)) {
+				error_line("Expected either a directory or a .odin file, got '%.*s'\n", LIT(fullpath));
+				return ParseFile_WrongExtension;
+			}
+		}
+		AstPackage *pkg = try_add_import_path(p, fullpath, fullpath, init_pos, Package_Normal);
+		if (pkg) {
+			pkg->is_extra = true;
+		}
+	}
+
 	thread_pool_start(&parser_thread_pool);
 	thread_pool_wait_to_process(&parser_thread_pool);
 
diff --git a/src/parser.hpp b/src/parser.hpp
index 48af0b293..aa288304e 100644
--- a/src/parser.hpp
+++ b/src/parser.hpp
@@ -150,6 +150,7 @@ struct AstPackage {
 	Scope *   scope;
 	DeclInfo *decl_info;
 	bool      used;
+	bool      is_extra;
 };