dune
dune copied to clipboard
Adding .gitignore and .ocamlformat to init #9567
Hi !
This is my very first time contributing to such a big open-source project in OCaml. As stated in the issue, that would be nice to have a .gitignore with with _build added as well as an empty .ocamlformat file once a project is created with dune init proj
This commit simply adds two files to the list of targets to create inside the proj function of the dune-init.ml Make module.
Related to: https://github.com/ocaml/dune/issues/9567
Hi, thanks for the contribution. As mentioned in #9567 we need to test the behavior when .gitignore and .ocamlformat already exist and in that case, presumably, keep the old content.
We just need a way to 'create if not exists, do nothing if exists'.
Hi, thanks for the contribution. As mentioned in #9567 we need to test the behavior when
.gitignoreand.ocamlformatalready exist and in that case, presumably, keep the old content.
Hi, the current behavior is this one when .ocamlformat and .gitignore files already exists.
$ dune init proj foo .
Warning: File .ocamlformat was not created because it already exists
Warning: File .gitignore was not created because it already exists
Success: initialized project component named foo
I bet what you mean is that we should avoid telling the dune init command to create those files so that warnings are gone ? Is that the expected behavior ?
The warnings actually make sense here, we are alerting the user of something unusual and perhaps unexpected. This is a good UX imho.
The warnings actually make sense here, we are alerting the user of something unusual and perhaps unexpected. This is a good UX imho.
Woups, just pushed some code that prevents creating the files if they already exists.
For reference, running cargo init . inside a folder already containing a .gitignore adds this to the file:
# Added by cargo
/target
When I'm then trying to make a cargo init . in a directory where a .gitignore file already have the /target directory inside, here is what the file become:
# Added by cargo
/target
# Added by cargo
#
# already existing elements were commented out
#/target
That would implies ""parsing"" the .gitignore file but seems like a usefull feature.
EDIT: I forgot to sign-off my last commit
@Julow wdyt? I thought .ocamlformat is no longer necessary in newer versions of ocamlformat
@gabyfle What I mean is that this PR should have an expect test that checks the behavior. The warning seems fine to me, but I would like to have the test to make sure that it 1) works and creates the files 2) doesn't overwrite the files if they already exist. This way we can be sure that this functionality is not broken by accident in future releases without us noticing.
@gabyfle What I mean is that this PR should have an expect test that checks the behavior. The warning seems fine to me, but I would like to have the test to make sure that it 1) works and creates the files 2) doesn't overwrite the files if they already exist. This way we can be sure that this functionality is not broken by accident in future releases without us noticing.
Ho ok, sure thing, I'll try to create a specific expect-test for this feature and push it here !
As someone using darcs + topiary for VCS + formatting respectively, I don’t know that these files should be automatically generated without a consenting flag, or a CLI [y/N], etc. …Or at least some logic that checks if there is a .git directory first. Something smells funny about endorsing specific tools which further ingrains status quo & adds an implicit endorsement for said tools therein.
Please keep comments specifically related to reviewing the PR. For general comments about the feature please comment on the related issue.
https://github.com/gabyfle/dune/pull/1