added sub command init for generating new project #178

GmosNM · 2023-11-14T00:00:21Z

closes #177

travisdoor

Thanks for your contribution, good job; however, there are some things to fix. Note that I made a new document for contributors here.

Please use clang-format before committing, the config can be found in the root of repo. Thanks.

travisdoor · 2023-11-14T12:25:45Z

src/main.c

+#ifdef _WIN32
+    #include <direct.h>
+    #define mkdir(path, mode) _mkdir(path)
+#endif
+#include <sys/stat.h>


This can be removed; please see the following comments.

travisdoor · 2023-11-14T12:26:27Z

src/main.c

@@ -118,10 +123,17 @@ typedef struct ApplicationOptions {
 	bool do_cleanup_when_done;
 } ApplicationOptions;

+
+typedef struct ProjectOptions{
+    bool is_executable;


This member is not used at all.

travisdoor · 2023-11-14T12:28:00Z

src/main.c

@@ -118,10 +123,17 @@ typedef struct ApplicationOptions {
 	bool do_cleanup_when_done;
 } ApplicationOptions;

+
+typedef struct ProjectOptions{


I use typedefs only for commonly used helper types; please see https://github.com/travisdoor/bl/wiki/Contribution#code-guidelines

travisdoor · 2023-11-14T12:29:00Z

src/main.c

 typedef struct Options {
 	ApplicationOptions     app;
 	struct builder_options builder;
 	struct target         *target;
+    ProjectOptions project;


Since only project_name is used here I suggest to rewrite this to char *init_project_name.

Or even better, you can remove it completely since you use this only as temporary later in case ID_INIT_PROJECT: branch. Project name can be local in the case branch.

travisdoor · 2023-11-14T12:30:04Z

src/main.c

@@ -245,6 +257,7 @@ void print_help(FILE *stream, struct getarg_opt *opts) {
 	                   "  blc [options] [source-files]\n\n"
 	                   "Alternative usage:\n"
 	                   "  blc [options] <-build> [build-arguments]\n"
+	                   "  blc [options] <--init> <project-name>\n"


We might make the --init option the first class, so -init should be used.

Hey, you've changed the arg implementation, but not the help comment, it should be -init [project-name] here.

travisdoor · 2023-11-14T13:32:48Z

src/main.c

+
+            fprintf(build_file, "\n\n\n%s", build_function_code);
+
+            mkdir("src", 0777);


Please use create_dir from common.h here.

travisdoor · 2023-11-14T13:34:24Z

src/main.c

+                "}\n";
+            fprintf(main_file, "%s", main_example);
+
+


Both files stay open; you should call fclose when file modification is done.

travisdoor · 2023-11-14T13:35:04Z

src/main.c

+
+            builder_info("INFO: project was initialize successfully");
+            builder_info("INFO: try blc -build");
+            return state;


Please use EXIT(EXIT_SUCCESS); instead to propper shutdown.

travisdoor · 2023-11-14T13:39:43Z

src/main.c

+            fprintf(build_file, "\n\n\n%s", build_function_code);
+
+            mkdir("src", 0777);
+            mkdir("bin", 0777);


Same as the previous one...

travisdoor · 2023-11-14T13:39:54Z

src/main.c

+
+            mkdir("src", 0777);
+            mkdir("bin", 0777);
+            FILE *main_file = fopen("src/main.bl", "w+");


Again missing error handling.

travisdoor

Thanks for the corrections, there is one important thing: We want to prevent users from accidentally override existing project. So before any modifications are done, we have to check whether there are src/main.bl or build.bl already present.

travisdoor · 2023-11-15T11:19:29Z

src/main.c

@@ -245,6 +257,7 @@ void print_help(FILE *stream, struct getarg_opt *opts) {
 	                   "  blc [options] [source-files]\n\n"
 	                   "Alternative usage:\n"
 	                   "  blc [options] <-build> [build-arguments]\n"
+	                   "  blc [options] <--init> <project-name>\n"


Hey, you've changed the arg implementation, but not the help comment, it should be -init [project-name] here.

travisdoor · 2023-11-15T11:21:39Z

src/main.c

+				project_name = argv[index + 1];
+			}
+
+			FILE *build_file = fopen("build.bl", "w+");


This is not fixed, BUILD_SCRIPT_FILE should be used instead of hardcoded filename.

travisdoor · 2023-11-15T11:24:46Z

src/main.c


 	struct getarg_opt optlist[] = {
+	    {
+	        .name = "-init",
+	        .help = "Creates a a project setup in your current folder."


Creates a a project -> Creates a project + missing space after folder.

travisdoor · 2023-11-15T11:29:01Z

src/main.c

+
+			char *exe_name = project_name;
+
+			fprintf(build_file, "\n\n\n");


No reason to have this IMHO.

travisdoor · 2023-11-15T11:30:50Z

src/main.c

+			fprintf(build_file, build_function_code_template, exe_name);
+
+			if (!create_dir("bin")) {
+				EXIT(EXIT_FAILURE);


Please add an error report instead of a silent failure. Same for this src creation.

travisdoor · 2023-11-15T11:40:21Z

src/main.c

+
+			FILE *main_file = fopen("./src/main.bl", "w+");
+			if (!main_file) {
+				builder_error("could not create build file!");


This is IMHO copy-paste, but the message is wrong?

travisdoor · 2023-11-15T11:42:16Z

src/main.c

+			                           "}\n";
+			fprintf(main_file, "%s", main_example);
+
+			builder_info("INFO: project was initialize successfully");


The INFO: prefix here might be redundant in the future, I generally don't do that. The builder_info can eventually do it automatically.

Typo: initialize -> initialized.

travisdoor · 2023-11-15T11:43:50Z

src/main.c

+			                                           "    compile(exe);\n"
+			                                           "}\n";
+
+			char *exe_name = project_name;


What's the point of this variable? You can use project_name directly.

travisdoor · 2023-11-15T11:49:11Z

src/main.c

@@ -673,6 +683,57 @@ int main(s32 argc, char *argv[]) {
 		case ID_RELEASE:
 			opt.target->opt = ASSEMBLY_OPT_RELEASE_FAST;
 			break;
+		case ID_INIT_PROJECT:
+			if (file_exists("build.bl")) {


This is not fixed, BUILD_SCRIPT_FILE should be used instead of hardcoded filename.

travisdoor · 2023-11-15T11:50:43Z

src/main.c

@@ -673,6 +683,57 @@ int main(s32 argc, char *argv[]) {
 		case ID_RELEASE:
 			opt.target->opt = ASSEMBLY_OPT_RELEASE_FAST;
 			break;
+		case ID_INIT_PROJECT:
+			if (file_exists("build.bl")) {
+				builder_error("File 'build.bl' exists in the current directory.");


Similar: builder_error("File '%s' exists in the current directory.", BUILD_SCRIPT_FILE);

It would be nice to have some descriptive message saying what is exactly wrong. Each error message should give the user some hint of how to fix it eventually or why it's reported as an error. What about: Current directory already contains initialized BL project. File '%s' exists.

travisdoor

There are still some minor issues, but I guess I can merge it and do the fixes later.

travisdoor · 2023-11-16T23:12:56Z

src/main.c

@@ -245,6 +246,7 @@ void print_help(FILE *stream, struct getarg_opt *opts) {
 	                   "  blc [options] [source-files]\n\n"
 	                   "Alternative usage:\n"
 	                   "  blc [options] <-build> [build-arguments]\n"
+	                   "  blc [options] <-init> <project-name>\n"


This should be <-init> [project-name]

travisdoor · 2023-11-16T23:14:36Z

src/main.c

+			char *project_name = "out";
+			if (argv[index + 1]) project_name = argv[index + 1];
+
+			FILE *build_file = fopen(BUILD_SCRIPT_FILE, "w+");


"w+" is not necessary here, you're not going to read from the file, so "w" should be enough.

gmos and others added 2 commits November 14, 2023 02:53

added sub command init for generating new project

000b9fa

better usage msg

48bae2b

travisdoor requested changes Nov 14, 2023

View reviewed changes

travisdoor added the enhancement New feature or request label Nov 14, 2023

GmosNM added 2 commits November 15, 2023 00:14

refactoring and error handling

a6b32af

added set_output_dir for the build script

854adc2

GmosNM requested a review from travisdoor November 14, 2023 21:25

GmosNM added 3 commits November 15, 2023 00:31

clang-format

461f1ae

making this work on windows

92fbbb8

removing extra buffer for the build function

d781c02

travisdoor requested changes Nov 15, 2023

View reviewed changes

GmosNM added 3 commits November 15, 2023 22:12

some fixes

b2d9009

handel if there is already main file in the current dir

b2f76e6

handel if there is already main file in the current dir

01af0f9

GmosNM requested a review from travisdoor November 16, 2023 20:59

travisdoor approved these changes Nov 16, 2023

View reviewed changes

travisdoor merged commit 5c5d630 into travisdoor:master Nov 16, 2023
6 checks passed

GmosNM deleted the init-command branch November 17, 2023 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added sub command init for generating new project #178

added sub command init for generating new project #178

GmosNM commented Nov 14, 2023 •

edited

Loading

travisdoor left a comment

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor Nov 15, 2023

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor Nov 14, 2023

travisdoor left a comment

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor Nov 15, 2023

travisdoor left a comment

travisdoor Nov 16, 2023

travisdoor Nov 16, 2023


		fprintf(build_file, "\n\n\n%s", build_function_code);

		mkdir("src", 0777);


		char *exe_name = project_name;

		fprintf(build_file, "\n\n\n");

added sub command init for generating new project #178

added sub command init for generating new project #178

Conversation

GmosNM commented Nov 14, 2023 • edited Loading

travisdoor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travisdoor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travisdoor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GmosNM commented Nov 14, 2023 •

edited

Loading