Support standard ` --ignore-matching-lines` parameter

davinkevin commented 8 months ago

Hello 👋 ,

First, I would like to thank you for your project, it's a good one, and I want to use it… thank you for it!

My "bug" report is about the standard parameter --ignore-matching-lines I would like to use to simplify the review process, especially on lengthy diff including a version number in files (yaml, for example).

For example, on this project, if I run the following command without difftastic:

❯ git diff d94cddf6b585649f2648c7deb317a1dfa62a4128~
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: monitoring/overlays/k8s-server/thanos/helmfile.yaml
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ monitoring/overlays/k8s-server/thanos/helmfile.yaml:14 @ releases:
      - ./values.yaml
    transformers:
      - ../../../../.base/components/helm/remove-helm-labels.yaml
      - ../../../../.base/components/helm/remove-kubernetes-version-label.yaml
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: monitoring/overlays/k8s-server/thanos/thanos.yaml
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ monitoring/overlays/k8s-server/thanos/thanos.yaml:12 @ metadata:
    app.kubernetes.io/instance: thanos
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: thanos
    app.kubernetes.io/version: 0.34.0
  name: thanos
  namespace: monitoring
---
@ monitoring/overlays/k8s-server/thanos/thanos.yaml:25 @ metadata:
    app.kubernetes.io/instance: thanos
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: thanos
    app.kubernetes.io/version: 0.34.0
  name: thanos-bucketweb
  namespace: monitoring
---
@ monitoring/overlays/k8s-server/thanos/thanos.yaml:38 @ metadata:
    app.kubernetes.io/instance: thanos
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: thanos
    app.kubernetes.io/version: 0.34.0
  name: thanos-compactor
  namespace: monitoring
---
@ monitoring/overlays/k8s-server/thanos/thanos.yaml:51 @ metadata:
    app.kubernetes.io/instance: thanos
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: thanos
    app.kubernetes.io/version: 0.34.0
  name: thanos-query
…
❯ git diff d94cddf6b585649f2648c7deb317a1dfa62a4128~ | wc -l
237

Almost all 237 lines are for the exact change, about the line app.kubernetes.io/version: 0.34.0.

If I use the standard git diff parameter to ignore this line, I have this:

❯ git diff --ignore-matching-lines 'app.kubernetes.io/version'  d94cddf6b585649f2648c7deb317a1dfa62a4128~
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: monitoring/overlays/k8s-server/thanos/helmfile.yaml
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ monitoring/overlays/k8s-server/thanos/helmfile.yaml:14 @ releases:
      - ./values.yaml
    transformers:
      - ../../../../.base/components/helm/remove-helm-labels.yaml
      - ../../../../.base/components/helm/remove-kubernetes-version-label.yaml

Which is precisely what I wanted.

However, If I run it with difftastic, the parameter is completely ignored.

❯ git diff --ignore-matching-lines 'app.kubernetes.io/version'  d94cddf6b585649f2648c7deb317a1dfa62a4128~
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: monitoring/overlays/k8s-server/thanos/helmfile.yaml
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ monitoring/overlays/k8s-server/thanos/helmfile.yaml:14 @ releases:
      - ./values.yaml
    transformers:
      - ../../../../.base/components/helm/remove-helm-labels.yaml
      - ../../../../.base/components/helm/remove-kubernetes-version-label.yaml
home-server on  main 36ms
❯ git diff --ignore-matching-lines 'app.kubernetes.io/version'  d94cddf6b585649f2648c7deb317a1dfa62a4128~
monitoring/overlays/k8s-server/thanos/helmfile.yaml --- YAML
File permissions changed from 100600 to 100644.
11 11       - ./values.yaml
12 12     transformers:
13 13       - ../../../../.base/components/helm/remove-helm-labels.yaml
   14       - ../../../../.base/components/helm/remove-kubernetes-version-label.yaml

monitoring/overlays/k8s-server/thanos/thanos.yaml --- 1/28 --- YAML
File permissions changed from 100600 to 100644.
 9  9     app.kubernetes.io/instance: thanos
10 10     app.kubernetes.io/managed-by: Helm
11 11     app.kubernetes.io/name: thanos
12 ..     app.kubernetes.io/version: 0.34.0
13 12   name: thanos
14 13   namespace: monitoring
15 14 ---

monitoring/overlays/k8s-server/thanos/thanos.yaml --- 2/28 --- YAML
23 22     app.kubernetes.io/instance: thanos
24 23     app.kubernetes.io/managed-by: Helm
25 24     app.kubernetes.io/name: thanos
26 ..     app.kubernetes.io/version: 0.34.0
27 25   name: thanos-bucketweb
28 26   namespace: monitoring
29 27 ---
…

Is it a bug or a thing not yet supported or not even planned?

Additional information:

❯ difft --version
Difftastic 0.54.0 (built with rustc 1.75.0)

I am running on macOS, which is installed with brew.

Wilfred commented 8 months ago

This isn't possible I'm afraid. --ignore-matchine-lines isn't passed to the external diff tool as far as I can see.

$ GIT_PAGER=cat DFT_LOG=trace GIT_EXTERNAL_DIFF=difft git diff --ignore-matching-lines 'app.kubernetes.io/version' --ext-diff
 2024-01-28T20:32:00.303Z INFO  difft::options > CLI arguments: ["src/conflicts.rs", "/tmp/git-blob-BI9V7X/conflicts.rs", "46602124597b3c0c7a20e89beee2a34ce9d376e1", "100644", "src/conflicts.rs", "0000000000000000000000000000000000000000", "100644"]

$ GIT_PAGER=cat DFT_LOG=trace GIT_EXTERNAL_DIFF=difft git diff --ext-diff                    
 2024-01-28T20:32:55.768Z INFO  difft::options > CLI arguments: ["src/conflicts.rs", "/tmp/git-blob-zpaXEJ/conflicts.rs", "46602124597b3c0c7a20e89beee2a34ce9d376e1", "100644", "src/conflicts.rs", "0000000000000000000000000000000000000000", "100644"]

Even if it was possible, I don't see how this feature could work for difftastic. Difftastic is diffing an abstract syntax tree (AST), and there's no way to apply a line-based exclude regex to an AST.

davinkevin commented 8 months ago

Thank you for your answer @Wilfred

This isn't possible I'm afraid. --ignore-matchine-lines isn't passed to the external diff tool as far as I can see.

Maybe something to report to the git project if you think it should be passed to the external diff tool, no?

Difftastic is diffing an abstract syntax tree (AST), and there's no way to apply a line-based exclude regex to an AST

I worked on AST for Java & Kotlin annotation processor, it was possible to filter out some cases when we had the AST representation. I don't want to presume about complexity of difftastic, but a pre or post processing of the value could be possible, if not using the AST directly?

0xdevalias commented 8 months ago

there's no way to apply a line-based exclude regex to an AST

Would it be possible to apply the line-based exclude after the AST-diffing occurs, when it is rendered back to a text format?

Wilfred / difftastic

Support standard ` --ignore-matching-lines` parameter #626