Wilfred / difftastic

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

Ignore trailing commas if they're not syntactically significant #419

Open nate-selzer opened 1 year ago

nate-selzer commented 1 year ago

Thanks for reporting a bug! Please include all of the following:

(1) A description of the issue. A screenshot is often helpful too.

When declaring a generator or typing method arguments over multiple lines, formatting libraries like black will add a trailing comma like so

def foo(
    long_variable_name: Optional[Bar[xyz, xyz]] = None,
    another_long_variable_name: Optional[Bar[xyz, xyz]] = None,
):

These are not syntactically significant, and it'd be nice if they could be ignored by difftastic.

Note that in cases where there is a single object inside parentheses, the comma is syntactically significant.

In the example below, foo is a string, bar is a generator.

foo = ("hi")
bar=("hello",)

Sidenote: it would also be nice if the parentheses around "hi" in the example above would be ignored.

Screen Shot 2022-11-04 at 4 13 11 PM

(2) A copy of what you're diffing. If you're diffing files, include the before and after files. If you're using difftastic with a VCS repository (e.g. git), include the URL and commit hash.

Can't share the whole thing as it's proprietary code, but comparing these 2 functions should work

def foo(
    long_variable_name: Optional[Bar[xyz, xyz]] = None,
    another_long_variable_name: Optional[Bar[xyz, xyz]] = None,
):
    pass
def foo(
    long_variable_name: Optional[Bar[xyz, xyz]] = None,
    another_long_variable_name: Optional[Bar[xyz, xyz]] = None
):
    pass

(3) The version of difftastic you're using (see difft --version) and your operating system.

Difftastic 0.37.0 MacOS Monterey 12.5.1

Delapouite commented 1 year ago

Hello.

This punctuation problem is mentioned in the doc. The example is about prettier but it also applies to similar beautifier tools like black in your case : https://difftastic.wilfred.me.uk/tricky_cases.html#autoformatter-punctuation

Wilfred commented 1 year ago

We can probably cover the simple cases by defining a list of tree-sitter syntax kinds where trailing commas don't matter. That seems nice to add, and would really help the formatting case.

I know that (1) and (1,) are different in Python, and other cases might exist. It'll need care.

Duologic commented 5 months ago

Would it be useful to add a CLI argument to ignore trailing commas? It could be a stop-gap solution to this problem.

Wilfred commented 2 months ago

Some cases to consider:

// ignore final comma
[1]
[1,]

// highlight final comma
[1,]
[1, 2,]

// possibly ignore first comma, especially if `1,` is on a separate line
[1]
[1, 2, 3]