GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — diff: use the largest context length given, like GNU diff by qodelicious · Pull Request #341 · uutils/diffutils

Conversation

@qodelicious

@qodelicious qodelicious commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

When more than one context length is given, GNU diff uses the biggest, and a bare -u or -c counts as 3. We kept the last number given and ignored a bare -u or -c, so -U1 -u gave 1 line of context instead of 3, and -U2 -U1 gave 1 instead of 2.

Now the parser keeps the largest one. Added a unit test with a few orders of -u/-U and -c/-C, checked against GNU diff 3.10.

Fixes #340

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sylvestre
sylvestre reviewed Nov 7, 2026
View reviewed changes
Comment thread src/params.rs Outdated
}

#[test]
fn largest_context_count_wins() {

@sylvestre sylvestre Nov 7, 2026

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move them to tests/by-util/test_diff.rs , with the -U1 -u case from the issue

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@qodelicious qodelicious Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved them to tests/by-util/test_diff.rs and added the -U1 -u case from the issue. The expected hunk headers match what GNU diff 3.10 prints for the same files.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/params.rs Outdated
(vec!["-C1", "-c"], 3),
(vec!["-c", "-C1"], 3),
(vec!["-C", "7", "-C2"], 7),
(vec!["--unified=1", "--unified"], 3),

@sylvestre sylvestre Nov 7, 2026

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please also cover --context=1 --context and the -5u form?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@qodelicious qodelicious Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added both, plus --context --context=1 and -5c. --context=1 --context ends up with 3 lines and -5u with 5, same as GNU.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sylvestre

sylvestre commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

thanks for working on this!
pre-commit.ci is still red, could you please have a look?
please also run the upstream test suite and tell us if it changes anything:
TESTS= ./tests/run-upstream-testsuite.sh
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
qodelicious added 2 commits November 10, 2026 22:41
@qodelicious
diff: use the largest context length given, like GNU diff
71cac32
@qodelicious
diff: move the context length tests to test_diff.rs
59741a1 
Cover the -U1 -u case from uutils#340, --context=1 --context and the -5u form,
with the expected hunks checked against GNU diff 3.10.
@qodelicious
qodelicious force-pushed the diff-context-largest-count branch from 74e4f83 to 59741a1 Compare November 10, 2026 22:45
@qodelicious

qodelicious commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

pre-commit.ci was red because of a merge conflict with main, not a lint error. Rebased and it's green now.

Ran the whole upstream suite on main and on this branch. Both give 19 pass, 10 fail, 4 skip, and the output is identical for every test. None of the upstream tests use two context lengths though, so the new test in test_diff.rs is what covers this.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sylvestre sylvestre sylvestre left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

diff: -U/-C given with -u/-c uses the wrong context length

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@qodelicious @sylvestre
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.