Wilfred / difftastic

a structural diff that understands syntax 🟥🟩
https://difftastic.wilfred.me.uk/
MIT License
21.1k stars 344 forks source link

Report 'no syntactic or textual changes' where appropriate #182

Open Wilfred opened 2 years ago

blag commented 2 years ago

This seems to be implemented. Or if it's not, it could use some additional details as to what else is still needed.

However, I actually don't think this is super useful, especially when linters intersect with pre-commit hooks.

Assume that you have code that is all committed, but needs linting. With a linter like the Black linter for Python, it will actually reformat your source code. But when you go to view the changes with difft, the only thing it shows you is "No syntactic changes":

airflow/cli/commands/db.py --- Python
No syntactic changes.

And while that is technically correct, it's not super useful. I still need to be able to view the non-syntactic changes made by my linter.

Now when you throw in pre-commit hooks into the picture, every time I go to make a commit, my pre-commit hooks run Black on all of my staged files. If one of my files was not completely linted, then Black will reformat a few files, and the pre-commit check will fail. Then when I go to view the unstaged changes with git diff, difftastic only shows me "No syntactic changes". Very much not useful for me, and this can happen for every commit.

When difftastic runs across a diff for a file that has no syntactic changes, I think it would be nice to emit a notification message at the top like "No syntactic changes" along with the parser that was used, while still displaying a normal diff underneath:

# Note: No syntactic changes (Python)
diff --git a/airflow/cli/commands/db.py b/airflow/cli/commands/db.py
index 4f8cca0ad..4f5ecf5c0 100644
--- a/airflow/cli/commands/db.py
+++ b/airflow/cli/commands/db.py
@@ -35,8 +35,9 @@ click_to_revision = click.option(
     '-r',
     '--to-revision',
     default=None,
-    help=("(Optional) If provided, only run migrations up to and including this revision. Note: must "
-          "provide either `--to-revision` or `--to-version`."
+    help=(
+        "(Optional) If provided, only run migrations up to and including this revision. Note: must "
+        "provide either `--to-revision` or `--to-version`."
     ),
 )
 click_to_version = click.option(

But besides this one small tweak, difftastic has been really great. Thank you for your efforts on this!

Wilfred commented 2 years ago

Yeah, I agree. I've actually made this mistake: accidentally reformatting some code and not immediately noticing because I used difftastic.

Long term I'd like to support this, but I'm still not happy with how the structural diffing works in all cases yet.