Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feture/new command structure #4

Merged
merged 6 commits into from
Dec 3, 2024
Merged

Conversation

CALEXCO
Copy link
Owner

@CALEXCO CALEXCO commented Dec 3, 2024

New command structure to be able to create new project and build the project

@CALEXCO CALEXCO marked this pull request as ready for review December 3, 2024 15:17
src/main.rs Outdated
#[derive(Subcommand, Debug, Clone)]
enum Commands {
/// Create new C Proejct with name
CNew {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with clippy here, I think prefixes in Enum variants are redundant.

Please remove the C prefix for each variant.

The project itself is named cinit and in the description/Readme describes that it is creating a C language tool, so it is not needed to prefix with c in functions variants and whatever.

src/main.rs Outdated
/// Build ./src/*.c files and moved to ./bin/
BuildC,
/// Build and run project
BulidCRunC,
}

#[derive(Clone)]
struct Cinit(Args);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove Prefix plz

}

fn c_new(project_name: &String, lib: Option<String>) -> Result<(), Box<dyn std::error::Error>> {
create_sub_directory(project_name, "src")?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the prefix from the name of the function

println!("Content written to {}", file_dir);
}
fn build_c() -> Result<(), std::io::Error> {
// Ejecutar GCC para compilar el programa
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove Prefix plz

src/main.rs Outdated
Comment on lines 60 to 63
.args(["src/main.c", "-Wall", "-o", "main"])
.status()
.expect("Failed to execute GCC");

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this command should be using the build system selected by the user.

If the user selects that wants a Makefile in the project I think this command should execute make

And you should consider the possibility of a different build system. For example nob , our dear friend tsoding is using it in their projects.

Copy link
Owner Author

Choose a reason for hiding this comment

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

For the moment the build command will only compile using gcc. Future versions will accept the build system selected by the user.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Put an issue and reference the issue in this conversation then

Copy link
Owner Author

@CALEXCO CALEXCO Dec 3, 2024

Choose a reason for hiding this comment

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

I have created the issue referring to your suggestion #6

src/main.rs Outdated
build_c()?;
//println!("{:?}", std::env::current_dir())
}
Commands::BulidCRunC => eprintln!("Developing"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can just use todo!() macro if this feature is not developed yet and the compiler will shut the fuck up

@CALEXCO CALEXCO merged commit 35a8208 into master Dec 3, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants