Skip to content

git: Add option to use difftastic as diff tool#2850

Merged
sumnerevans merged 1 commit intonix-community:masterfrom
notarock:master
Apr 4, 2022
Merged

git: Add option to use difftastic as diff tool#2850
sumnerevans merged 1 commit intonix-community:masterfrom
notarock:master

Conversation

@notarock
Copy link
Copy Markdown
Contributor

Description

Add the option to use difftastic, a syntax-aware diff tool, with git

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@notarock notarock requested a review from rycee as a code owner March 30, 2022 02:07
Comment thread modules/programs/git.nix Outdated
@notarock
Copy link
Copy Markdown
Contributor Author

notarock commented Mar 30, 2022

Pretty sure it would be good to only allow to enable one of either difftastic or delta but I am not too familiar with nix, how can I achieve this?

Comment thread modules/programs/git.nix
Comment thread modules/programs/git.nix Outdated
@notarock notarock force-pushed the master branch 5 times, most recently from 16d6d55 to 5d134df Compare March 30, 2022 05:31
@notarock notarock requested a review from sumnerevans March 30, 2022 05:32
Comment thread modules/programs/git.nix Outdated
Copy link
Copy Markdown
Contributor

@sumnerevans sumnerevans left a comment

Choose a reason for hiding this comment

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

sorry, one more thing

Comment thread modules/programs/git.nix Outdated
@sumnerevans
Copy link
Copy Markdown
Contributor

Also, please squash your commits to a single one that follows the contributing guide for commit messages.

Comment thread modules/programs/git.nix
Comment thread modules/programs/git.nix Outdated
@notarock
Copy link
Copy Markdown
Contributor Author

notarock commented Apr 2, 2022

Thanks for the review, I fixed it. Sorry for the constant back-and-forth, hopefully this is good now 😅

Comment thread modules/programs/git.nix Outdated
Comment thread modules/programs/git.nix Outdated
Difftastic is a syntax-aware diff tool which can be used with git.
@notarock notarock requested a review from sumnerevans April 4, 2022 14:33
@sumnerevans sumnerevans merged commit 0382c5f into nix-community:master Apr 4, 2022
@sumnerevans
Copy link
Copy Markdown
Contributor

Thanks! difftastic seems nice

jficz pushed a commit to jficz/home-manager that referenced this pull request Apr 7, 2022
Difftastic is a syntax-aware diff tool which can be used with git.
@lobre
Copy link
Copy Markdown
Contributor

lobre commented Jul 6, 2022

Hello @notarock,

Is there for convenience that you set the pager in that MR? I don’t see it mentioned in the documentation.

core.pager = "${pkgs.less}/bin/less -XF";

https://difftastic.wilfred.me.uk/git.html

I like to set my own core.pager to less -+FX to enable mouse scroll. But when I am enabling difftastic, it conflicts and I cannot define my own config anymore.

Is it possible to keep that option free for the end user?

@sumnerevans
Copy link
Copy Markdown
Contributor

Maybe we need to do core.pager = mkDefault ...?

@notarock
Copy link
Copy Markdown
Contributor Author

notarock commented Jul 7, 2022

Is there for convenience that you set the pager in that MR?

No particular reasons really, it just made the output play nice enough for me when I first contributed the module.

@lobre
Copy link
Copy Markdown
Contributor

lobre commented Jul 8, 2022

No particular reasons really, it just made the output play nice enough for me when I first contributed the module.

@notarock, would you agree to having it removed, or maybe using mkDefault to avoid forcing it on users?

@notarock
Copy link
Copy Markdown
Contributor Author

would you agree to having it removed, or maybe using mkDefault to avoid forcing it on users?

I don't have an opinion on this. Making it configurable with a default value seems like the way to go, but I don't have time to submit a pull request at this time. Feel free to open one if needed

@lobre
Copy link
Copy Markdown
Contributor

lobre commented Jul 12, 2022

I decided to stick with the default git diff algorithm at the end instead of using difftastic. Making it configurable is the solution, but I will also let that duty of creating a PR to anyone requiring it in the future.

Thanks anyway for helping with that.

@teto teto mentioned this pull request Aug 22, 2022
7 tasks
teto pushed a commit to teto/home-manager that referenced this pull request Aug 22, 2022
Difftastic is a syntax-aware diff tool which can be used with git.
spacekookie pushed a commit to spacekookie/home-manager that referenced this pull request Feb 10, 2023
Difftastic is a syntax-aware diff tool which can be used with git.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants