v.util: rewrite diff module, deprecate old functions

[ttytm]

The rewritten module should extend functionality, make usage easier/less verbose, fix issues and resolve friction points that may stand in the way of using it in whatever context.

Some of the changes:

An overview of the changes is given below. However, it should not be necessary for the review, it was attempted to keep the code changes compact and easy to understand. Feel free to skip the overview.

General reliability and predictability of return values:

• Use of known diff tools that can actually return a string value.

  To make the function call itself as safe and reliable as possible, tools like code and opendiff that open non-conforming files in a gui and do return a result string is limited to the overwrite via the environment variable. Additionally, it is possible to disable and to customize overwrites via the env variable.

Error handling:

• Use of Result return types
  • The "old" module handles errors via unrecoverable panics or returns them formatted as string.
    Both should be resolved through using !string return types.

Other changes:

• Fix \ No newline at end of file in the return values of the string comparing function.
  Which is suboptimal for a function that by definition compares strings.

  // Current
  diff.color_compare_strings(diff.find_working_diff_command()!, 'unique_smthn', 'abc', 'cde')
  // New
  diff.compare_text('abc', 'cde')!

  Comparing the output (assuming the automatically available diff command is diff)

  # Current
  ❯ v run file.v
  # ...
  -abc
  \ No newline at end of file
  +cde
  \ No newline at end of file
  
  # New
  ❯ v run file.v
  # ...
  -abc
  +cde

• Include delta as diff tool. Which is a feature rich tool for human readable diffs that can help to analyze outputs. It can also emulate diff-so-fancy.

• Extend tests.

• For now, vfmt.v was updated as the only module that the diff module internally - also to make it easier testing the changes during a review locally. Other that use the current diff module could be updated separately.

Better handled, more user oriented options:

• Handle options in a non-conflicting way. The current module has some defaults options that are hard coded to be always added, which might conflict with other args or tools.

  • This is also a fix for the statement in v fmt --help, which shows VDIFF_OPTIONS="-W 80 -y" (using the -y flag for side-by-side comparison).

    Current	Updated

    As a reference to verify that it's not a regression 87270e9 is a commit before the recent fixes and preparations of the diff module.

  • Allows to use other tools

    Current	Updated

[spytheman]

The default should not change the content of what is passed.

The No newline at end of file message is a valuable signal.

[ttytm]

Can you add more info why? This would help me agree.

I would agree immediately if we would compare files. But this is comparing strings and the No newline at end of file seems inappropriate when comparing strings.

[ttytm]

Or let me phrase it like this. I want to make aware that this is comparing strings.
So No newline at end of file would be in the output for most of comparisons made. If you are aware and this is the preferred way, I have no trouble updating this.

[spytheman]

Consider the most frequent usage - comparing text, generated by vfmt, to input source, that was written by a human, using an unknown text editor.
The input may not have an ending line. The output will.

Consider also that such diffs, will be used for assert txt1 == txt2.
When those 2 differ, it is very valuable to know what the exact difference was, and that is easier to discern, when you know that the tool does not lie to you, by manipulating and hiding data.

[ttytm]

Hmm, I still think there can be a misunderstanding here - on both sides. So lets try to get things clear.

There still is compare_files. There the + '\n' isn't done and it's the function used by v fmt (it's the only one updated in this PR). It still prints No newline at end of file if it is the case for the formatted file.

In regards of asserting, how there will be a difference? the + '\n' is added to both asserted strings, and it is only added for "quasi"-formatting-reasons of the colored diff output, so that the result resembles the function signature better.

[ttytm]

However, shouldn't the newline only be added if there isn't one there already? Otherwise, you could end up with an extra on one, adding a difference that wasn't in the original.

Not necessarily, as it is added to both texts it is not a difference that the tools will track and highlight as a difference. So is will not change what is highlighted in the output.

I temporarily had a condition for this before opening the PR but thought to skip the additional check when it doesn't create a diff. Now thinking again, re-adding it could remove one additional non-differing empty line after the diffs, depending on the used configuration.

To give examples of what is highlighted with things as they currently are:

println(diff.compare_text('abc\n\n', 'cde\n', tool: .colordiff)!)

println(diff.compare_text('abc\n\n', 'cde\n', tool: .colordiff, args: '-y -W 80')!)

println(diff.compare_text('abc\n\n\n', 'cde\n\n')!)

[JalonSolov]

That extra "non-differing empty line" is what I was talking about. If it isn't there in the original text, it shouldn't show up in the output, either.

[ttytm]

Agreed, and added it. It ensures a cleaner visualization when using custom options. The results of the comparison itself that shows the differences stays ofc the same.

[ttytm]

Having had a long day yesterday and other things to close off, I just wanted to get done with things in the PR. But I see, that I wanted to go to fast here. This PR is a small "bigger" change. So it's okay to go as slow as required.

To get the best result. I'll re-add preventing of \ No newline at end of file, as I think it's the right thing to do for that function. Then we can continue discussing it. If there should be any concerns with that, it would help me to address them if you test the function locally. Maybe this resolves concerns in itself or you can find an example and we would have something concrete to discuss.

[spytheman]

How is VDIFF_TOOL and VDIFF_OPTIONS supported in the new code?

From what I can see, they are only read now in find_working_diff_command, which is deprecated in this PR after 2024-06-30.

What will happen, when it expires?

[spytheman]

Why is find_working_diff_command deprecated at all, if there is not a suitable replacement, offering the same functionality?

[ttytm]

Why is find_working_diff_command deprecated at all, if there is not a suitable replacement, offering the same functionality?

Because it is not required. Or do I miss something?
As far as I could see, there isn't a need to make it necessary to use a module function to find the diff command to then pass it to another module function. This can be handled internally. So there is only an internal command for this.

// master
diff.color_compare_strings(diff.find_working_diff_command()!, 'unique_smthn', 'abc', 'cde')

// updated
diff.compare_text('abc', 'cde')!

or

// master
diff_cmd := diff.find_working_diff_command()!
// ...
diff.color_compare_files(diff_cmd, file_path_1, file_path2)

// updated
diff.compare_files(file_path1, file_path_2)!

There is no need to manually search for a diff command to then pass it to one of that functions. If there should ever be the need to check for the automatically found, available diff tool, it is still available as public constant available_tool.

What will happen, when it expires?

It can removed then since it's not required.

[ttytm]

How is VDIFF_TOOL and VDIFF_OPTIONS supported in the new code?

They are part of the old code and replaced by an alternative.

By default, it is VDIFF_CMD that can be used to pass a custom tool and options.

To give developers full control, and make the module attractive to use anywhere, the env variable itself is also configurable as part of CompareOptions.
The used variable can be customized or set to none to disable overwrites, in case developers don't want to allow to overwrite values that they are passing to the diff functions.

@[params]
pub struct CompareOptions {
pub:
	// ...
	// Sets the environment variable whose value can overwrite a diff command passed to a compare function.
	// It also enables the use of commands that are not in the list of known diff tools.
	// Set it to `none` to disable it.
	env_overwrite_var ?string = 'VDIFF_CMD'
}

Internally, the new variable is handled in a way that should better support custom options.

E.g. current master (this is stated to be working in v fmt --help, but doesn't)

VDIFF_TOOL="diff" VDIFF_OPTIONS="-W 120 -y" v fmt -diff file.v

No issues with the new command

VDIFF_CMD="diff -W 120 -y" v fmt -diff file.v

Screenshots and another example for this are in the last section of the PRs description.

Of course, when starting to use the new functions, documentation should be updated in the corresponding locations, like v fmt --help.

[ttytm]

It is intended that everything is intuitive to use, please bare with me when explanations make things seem complicated. I'm open for questions if there are more concerns.