@baile320 Thanks a lot for your proposal! 🔥

1. I just used a Unix epoch time timestamp. We could add a more nicely formatted one, but it seems likely we'd need to add a crate (I'm not sure?), and I don't know if that's wanted or needed. The Unix timestamps still allow the copies to easily be sorted by most recent.

I think the Unix timestamps will be enough for the first time.

2. Should copying the file be automatic any time there are fixes?

   1. If it should be automatic, should there be an opt-out flag? If so, what flag? -n//--no-copy?
   2. If users should always specify a flag to create a copy, should we prompt them that their file will get overwritten?

I think this feature should be by default when a user tries to fix warnings and we should add an argument to skip this action.

Also, I have some proposals:

1. This feature should be named backup;
2. The flag to skip this action should be named --no-backup;
3. We should inform users about creating a backup copy of files after fixing.

Thanks for your comments, I will make those changes and convert this from a draft when I'm finished 👍

Just an update/FYI - I haven't forgotten about this, just been busy. I think I'll have time do make the changes later today.

I believe this should be ready for review now. Changes made since discussion:

1. Renamed feature from Copy to Backup (changed arg to "no-backup", --no-backup, and no short name)
2. Using fs::copy instead of fs::write to create the file.
3. Inform users of the file created after creating it.
4. Only back up file if the .env file is being modified
5. Added the backup files to .gitignore as well (.env_)

I'm not sure why the CI / Test and coverage / Linux (pull_request) Failing after 45s — Test and coverage / Linux is failing, is this something I need to look into? I see a couple other PRs where it failed as well.

I'm not sure why the CI / Test and coverage / Linux (pull_request) Failing after 45s — Test and coverage / Linux is failing, is this something I need to look into?

This is due to a change in the output:
https://github.com/dotenv-linter/dotenv-linter/pull/272/checks?check_run_id=1047857184#step:9:210

This is due to a change in the output:
https://github.com/dotenv-linter/dotenv-linter/pull/272/checks?check_run_id=1047857184#step:9:210

Please also update all integration tests and write another one to cover the --no-backup flag.

Codecov Report

Merging #272 into master will decrease coverage by 0.18%.
The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master     #272      +/-   ##
==========================================
- Coverage   97.64%   97.46%   -0.19%     
==========================================
  Files          31       31              
  Lines        2634     2679      +45     
==========================================
+ Hits         2572     2611      +39     
- Misses         62       68       +6     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a86e76...a7bc286. Read the comment docs.

Please also update all integration tests and write another one to cover the --no-backup flag.

Still need to complete this task, will get to it later today and post a reply when completed

hmm... i'm not sure how much detail I should be checking when a backup is created in an integration test. since the file is created with a timestamp, i'm not sure how to know when testing what that timestamp would be? I could just check for a wildcard file or something if that's necessary? I feel a little stuck on testing these fs components.

but in the unit test i already do make sure the filepath & backup filepath are different, and that the backup filepath has the original environment.

hmm... i'm not sure how much detail I should be checking when a backup is created in an integration test. since the file is created with a timestamp, i'm not sure how to know when testing what that timestamp would be? I could just check for a wildcard file or something if that's necessary? I feel a little stuck on testing these fs components.

but in the unit test i already do make sure the filepath & backup filepath are different, and that the backup filepath has the original environment.

In my opinion, in the intragration test you should create a file with some problems, for example: foo=bar.
Then, you should run dotenv-linter with the -f flag and check that:

1. The content of the .env file doesn't equal foo=bar;
2. The content of the backup file equal foo=bar;

I think that is all.

Hm. I think having a separate function for a single test is probably overkill as well, but I think I need a testing function that has some different functionality than the current ones.

I think I need a test function that will let me

1. supply arguments
2. not close/delete TestDir when done, and doesn't take ownership
3. doesn't require me to supply the expected output.

The reasons for those are:

1. I changed the other tests to have -f --no-backup (I think this is OK, it basically acts to preserve the functionality of the tests that existed prior to adding this feature),
2. I need to be able to the examine the TestDir for the backup file after running
3. I don't know what the expected output will be because we're generating a dynamic timestamp every time we run, and I don't see an easy way of mocking that value.

I'm open to suggestions on any of these points, especially the last one.

Hm. I think having a separate function for a single test is probably overkill as well, but I think I need a testing function that has some different functionality than the current ones.

I think I need a test function that will let me

1. supply arguments
2. not close/delete TestDir when done, and doesn't take ownership
3. doesn't require me to supply the expected output.

The reasons for those are:

1. I changed the other tests to have -f --no-backup (I think this is OK, it basically acts to preserve the functionality of the tests that existed prior to adding this feature),
2. I need to be able to the examine the TestDir for the backup file after running
3. I don't know what the expected output will be because we're generating a dynamic timestamp every time we run, and I don't see an easy way of mocking that value.

I'm open to suggestions on any of these points, especially the last one.

👌 Let it be 🙂

@baile320 Thanks a lot! You're awesome! ❤️