Lewis Russell

unread,

Dec 13, 2021, 11:39:21 AM12/13/21

to vim/vim, Subscribed

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/9344

Commit Summary

2c989d8 xdiff: Use hunk_func instead of out_line

File Changes

(1 file)

M src/diff.c (182)

Patch Links:

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

Lewis Russell

unread,

Dec 13, 2021, 11:41:42 AM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

7caf96f xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 11:43:51 AM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

9fc50a6 xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 11:45:36 AM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

bdc1dbd xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 11:47:20 AM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

05eb544 xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 11:49:48 AM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

2925c6b xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 11:54:21 AM12/13/21

to vim/vim, Subscribed

@lewis6991 commented on this pull request.

In src/diff.c:

> -	    //
-	    // unified diff looks like this:
-	    // --- file1       2018-03-20 13:23:35.783153140 +0100
-	    // +++ file2       2018-03-20 13:23:41.183156066 +0100
-	    // @@ -1,3 +1,5 @@
-	    if (isdigit(*line))
-		diffstyle = DIFF_ED;
-	    else if ((STRNCMP(line, "@@ ", 3) == 0))
-	       diffstyle = DIFF_UNIFIED;
-	    else if ((STRNCMP(line, "--- ", 4) == 0)
-		    && (vim_fgets(linebuf, LBUFLEN, fd) == 0)
-		    && (STRNCMP(line, "+++ ", 4) == 0)
-		    && (vim_fgets(linebuf, LBUFLEN, fd) == 0)
-		    && (STRNCMP(line, "@@ ", 3) == 0))
-		diffstyle = DIFF_UNIFIED;
+	    if (fd == NULL)

Diff from here to line 1730 is unchanged and just indented.

Lewis Russell

unread,

Dec 13, 2021, 11:55:41 AM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

fd72060 xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 11:56:52 AM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

54ca960 xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 11:58:04 AM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

41c08cf xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 12:00:41 PM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

5939572 xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Lewis Russell

unread,

Dec 13, 2021, 12:06:17 PM12/13/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

45c4fdc xdiff: Use hunk_func instead of out_line

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Yegappan Lakshmanan

unread,

Dec 13, 2021, 12:32:15 PM12/13/21

to vim_dev, nor...@github.com, vim/vim, Push

Hi,

On Mon, Dec 13, 2021 at 9:06 AM Lewis Russell <vim-dev...@256bit.org> wrote:

@lewis6991 pushed 1 commit.

45c4fdc xdiff: Use hunk_func instead of out_line

Note that each update to your pull request triggers a lot of CI tests in the backend:

https://github.com/vim/vim/actions

https://ci.appveyor.com/project/chrisbra/vim/history

https://cirrus-ci.com/github/vim/vim

So instead of updating the PR with a lot of minor changes, can you push the changes

once you have locally tested them and it is ready (to minimize the number of updates

to the PR)?

Regards,

Yegappan

Bram Moolenaar

unread,

Dec 13, 2021, 5:06:56 PM12/13/21

to vim/vim, Subscribed

@chrisbra

Christian Brabandt

unread,

Dec 14, 2021, 2:38:44 PM12/14/21

to vim/vim, Subscribed

very nice. So instead of calling the xdiff-callback for each line, we only call it per hunk. I like it.

Small nit:

Instead of adding a separate argument to diff_read like this:

diff_read(idx_orig, idx_new, &dio->dio_diff, dio->dio_internal);

I would just pass down the dio structure.

Also I noticed there are several test failures, that indicates that the diffing currently does not work? So it doesn't really look ready yet? Check Test_diff_filler() for a simple test that fails with the expected number of filler lines.

Dominique Pellé

unread,

Dec 14, 2021, 3:11:24 PM12/14/21

to vim/vim, Subscribed

@dpelle commented on this pull request.

In src/diff.c:

> -	    emsg(_("E959: Invalid diff format."));
-	    break;
+	    if (diffstyle == DIFF_NONE)
+	    {
+	        // Determine diff style.
+	        // ed like diff looks like this:
+	        // {first}[,{last}]c{first}[,{last}]
+	        // {first}a{first}[,{last}]
+	        // {first}[,{last}]d{first}
+	        //
+	        // unified diff looks like this:
+	        // --- file1       2018-03-20 13:23:35.783153140 +0100
+	        // +++ file2       2018-03-20 13:23:41.183156066 +0100
+	        // @@ -1,3 +1,5 @@
+	        if (isdigit(*line))
+	    	diffstyle = DIFF_ED;

Indentation looks inconsistent with the indentation in else block.

Lewis Russell

unread,

Dec 15, 2021, 1:33:21 PM12/15/21

to vim/vim, Subscribed

very nice. So instead of calling the xdiff-callback for each line, we only call it per hunk. I like it.

Small nit:

Instead of adding a separate argument to diff_read like this:
diff_read(idx_orig, idx_new, &dio->dio_diff, dio->dio_internal);
I would just pass down the dio structure.

Sure will do.

Also I noticed there are several test failures, that indicates that the diffing currently does not work? So it doesn't really look ready yet? Check Test_diff_filler() for a simple test that fails with the expected number of filler lines.

The offsets applied are probably slightly wrong. I adapted this from Lua which has 1-indexing.

Lewis Russell

unread,

Dec 21, 2021, 6:50:23 AM12/21/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

2a8b7e6 xdiff: Use hunk_func instead of out_line

—
View it on GitHub.

Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Lewis Russell

unread,

Dec 21, 2021, 6:59:05 AM12/21/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

935b059 xdiff: Use hunk_func instead of out_line

—
View it on GitHub.

Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Lewis Russell

unread,

Dec 21, 2021, 7:10:11 AM12/21/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

1aeab9c xdiff: Use hunk_func instead of out_line

—
View it on GitHub.

Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Lewis Russell

unread,

Dec 21, 2021, 7:27:02 AM12/21/21

to vim/vim, Push

@lewis6991 pushed 1 commit.

b8a2c93 xdiff: Use hunk_func instead of out_line

—
View it on GitHub.

Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

codecov[bot]

unread,

Dec 21, 2021, 7:39:35 AM12/21/21

to vim/vim, Subscribed

Codecov Report

Merging #9344 (2418236) into master (1e78deb) will decrease coverage by 87.86%.
The diff coverage is 0.07%.

❗ Current head 2418236 differs from pull request most recent head b8a2c93. Consider uploading reports for the commit b8a2c93 to get more accurate results

@@             Coverage Diff             @@

##           master    #9344       +/-   ##

===========================================

- Coverage   90.32%    2.45%   -87.87%     

===========================================

  Files         151      152        +1     

  Lines      172721   167652     -5069     

===========================================

- Hits       156004     4118   -151886     

- Misses      16717   163534   +146817

Flag	Coverage Δ
huge-clang-none	`?`
huge-gcc-none	`?`
huge-gcc-testgui	`?`
huge-gcc-unittests	`2.45% <0.07%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/autocmd.c	`2.02% <0.00%> (-90.45%)`	⬇️
src/blob.c	`0.00% <0.00%> (-92.60%)`	⬇️
src/buffer.c	`3.45% <0.00%> (-89.28%)`	⬇️
src/dict.c	`5.47% <0.00%> (-86.89%)`	⬇️
src/diff.c	`0.00% <0.00%> (-91.19%)`	⬇️
src/digraph.c	`0.00% <0.00%> (-96.38%)`	⬇️
src/drawscreen.c	`1.62% <0.00%> (-85.15%)`	⬇️
src/edit.c	`0.00% <0.00%> (-93.18%)`	⬇️
src/eval.c	`0.10% <0.00%> (-95.93%)`	⬇️
src/evalfunc.c	`0.00% <0.00%> (-96.54%)`	⬇️
... and 175 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e78deb...b8a2c93. Read the comment docs.

—

Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Dec 28, 2021, 4:57:03 AM12/28/21

to vim/vim, Subscribed

so, is this ready?

—

Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Lewis Russell

unread,

Dec 28, 2021, 4:59:44 AM12/28/21

to vim/vim, Subscribed

Yes this should be good for another review.

I fixed all the related CI failures. The remaining look unrelated to me.

—

Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Dec 28, 2021, 6:55:37 AM12/28/21

to vim/vim, Subscribed

@chrisbra let me know if this should be included now.

—

Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Dec 28, 2021, 7:46:12 AM12/28/21

to vim/vim, Subscribed

Yes, can be included now. Note: I had some problems applying the patch when using the patch file: https://patch-diff.githubusercontent.com/raw/vim/vim/pull/9344.patch (git complained about some unwanted whitespace changes as well as an un-parseable diff header. Using the diff file however worked: https://patch-diff.githubusercontent.com/raw/vim/vim/pull/9344.diff without any problems.

—

Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Dec 28, 2021, 8:55:27 AM12/28/21

to vim/vim, Subscribed

Closed #9344 via d9da86e.

—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward

[vim/vim] xdiff: Use hunk_func instead of out_line (PR #9344)

Lewis Russell

You can view, comment on, or merge this pull request online at:

Commit Summary

File Changes

Patch Links:

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Yegappan Lakshmanan

Bram Moolenaar

Christian Brabandt

Dominique Pellé

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

Lewis Russell

codecov[bot]

Codecov Report

Christian Brabandt

Lewis Russell

Bram Moolenaar

Christian Brabandt

Bram Moolenaar