cruft icon indicating copy to clipboard operation
cruft copied to clipboard

Feature request: cruft diff should optionally compare whole project

Open HappyEinara opened this issue 3 years ago • 5 comments

In cruft diff here, cruft copies project files that also exist in the hydrated template.

Early in a template's life I'm developing it by using a toy project, adding tests, ci etc, checking they work and then merging the results into the template with the appropriate jinja when each change is good enough. I presume this is a common pattern for you too.

The problem is that cruft diff won't pick up any files I add, only changes in files that currently exist in the template.

My suggestion is to add a cli option to include new files.

A couple of extra points:

  • Obviously when the work being done is development on an application and not the template itself, this option is of no use nor interest. My suggestion is just a way to help iterate over the development of a template itself.
  • Typically there will be junk (cruft, ha!) that is irrelevant, stuff like .git, .mypy_cache, **/__pycache__. Those files won't be relevant to the template. That's probably a good justification for the current implementation. However a quick test suggested to me that git diff (which cruft uses under the hood) respects .gitignore. This seems to be true even when --no-index is used and the two project dirs are not git repos. I might be mistaken, but if I'm right then implementing my idea should be pretty easy; just make the new flag toggle from remote_template_dir to local_template_dir at L49

HappyEinara avatar Apr 02 '21 07:04 HappyEinara

I'm now working on this.

My comments on just letting git diff respect the .gitignore were off base; I think I was wrong about it working with --no-index. However, that's moot because the kind of stuff that gets filtered out of a project directory by .gitignore can be pretty big. .tox, .mypy_cache, python bytecode and .git itself all need filtering before copying to the tmpdir. But it's workable anyway, I think, and I certainly have a need for my solution.

You can assign the issue to me if you like.

HappyEinara avatar Apr 15 '21 09:04 HappyEinara

Little update: this is coming along well. My POC works and I'm already excited about using it for my own templates.

Working through a couple of bugfixes to get the support for #99 working and tested, and then I need to make sure the code is a pleasure to read before I submit the PR.

HappyEinara avatar Apr 26 '21 22:04 HappyEinara

@HappyEinara any updates on this?

sambhav avatar May 22 '21 12:05 sambhav

@samj1912 Apologies for the late reply. I lost momentum a little on this because life got in the way: i have some urgent tax work to complete and the attendant anxiety to manage. The status is approximately the same: reverse diff works. I have a minor bug to diagnose in the "pass explicit paths" piece (which ties in with the reverse diff quite neatly). Apart from that I had to rewrite bits of the diff command and I need to refactor it a bit to make it readable and consistent with the existing design of the codebase.

tl;dr bear with me, it's not many more coding hours but I really have to take care of the life stuff. ETA maybe a couple of weeks depending on my success there.

HappyEinara avatar Jun 05 '21 03:06 HappyEinara

Is this the same reason why cruft diff and cruft update (it also has view diff mode) produce different diff? If yes, then have we considered --dry-run flag in cruft update command instead which simply prints the diff?

@HappyEinara any update? If you need help, perhaps you can send me a link to your fork so I can take your patch forward.

TBH I find it unintuitive that the default behaviour of cruft diff does not care about the contents of the project dir.

smitthakkar96 avatar Jan 03 '23 13:01 smitthakkar96