ymattw / ydiff

View colored, incremental diff in workspace or from stdin with side by side and auto pager support
Other
873 stars 62 forks source link

Add 'force pager' option #56

Closed hypnoglow closed 8 years ago

hypnoglow commented 8 years ago

Use case: I use cdiff as a difftool. This is what I have in .gitconfig:

[diff]
    tool = cdiff
[difftool "cdiff"]
    cmd = cdiff -s -c always -w 0 \"$LOCAL\" \"$REMOTE\"
[difftool]
    prompt = false

The problem is when my diff consists of many files where I changed a few lines of code. The difftool will print that files NOT using the pager, but as cat would. So I have to scroll my terminal up to see the start of diff. This is caused by less feature when -F flag is passed to it.

This patch can allow me to use

[difftool "cdiff"]
    cmd = cdiff -s --force-pager -c always -w 0 \"$LOCAL\" \"$REMOTE\"

So the output of difftool will be always paged.

ymattw commented 8 years ago

Thanks for the PR. Sorry I couldn't respond earlier (still in a place with limited internet access). I didn't expect people to use cdiff like this because it's designed to be invoked in a VCS workspace directly (typing cdiff is faster than typing git difftool). But I admit your use case is valid.

My concern so far is, is the less option -+F widely available? Could you help look into its change log and the versions in major distributions? Updating the document as well would be very welcome.

hypnoglow commented 8 years ago

I have done some research, what I found:

The -+ command was implemented quite long ago: I have found a statement "New command -+ sets an option to its default." under section named "Major changes between "less" versions 123 and 170" in the file named NEWS in the source code of less (downloaded from here http://www.greenwoodsoftware.com/less/download.html ).

The -F option appeared in LESS version 346 in 1999. http://www.greenwoodsoftware.com/less/news.346.html

Anyway, If you are confused, passing no F option instead of -+F also works, so I can rework this.

Also, I am a bit not sure now, because the behavior changes slightly with other options provided/available.

  1. This is current behavior on difftool (less -RSXF): option1
  2. This is what this pull request does (less -RSX): option2
  3. Another option (less -RS): option3

So, my question is, should we do something with this? We could allow passing an argument to cdiff where we can specify any less options. This would be very flexible, but somewhat overcomplicated, and I agree that your tool should be simple yet functional. Or we just keep one simple parameter (like I have already done) to make what we want (to provide less -RSX or (less -RS).

What is your opinion on this?

ymattw commented 8 years ago

Thanks for the update and nice animated screenshots.

My understanding is that git difftool would run the diff tool on the changed files one by one, so -

Case 1:less -RSXF

The -F option means --quit-if-one-screen, it does not ask you to press q to quit, that says, if all your changes are small enough to fit in one screen, you will never see the pager in action.

Case 2: less -RSX

Now you removed -F so less will always ask for pressing q.

Case 3: less -RS

Option -X is useful to prevent termcap deinitialization which usually clears the screen, you removed it so you see a clean screen for each of you files in the change list.

Conclusion

All behaviour we see here is normal and expected. I suggest you update your alias gdt to use cdiff directly, it should give you a better pager experience.

If you have to use it the 'git difftool' way, update the [difftool "cdiff"] section with a LESS environment like below, cdiff respects the environment and won't force the -F if it's defined.

[difftool "cdiff"]
    cmd = LESS=-SXR cdiff -s -c always -w 0 \"$LOCAL\" \"$REMOTE\"

Question

What's the tool you use to record the animation? It's much better than Recordit I am using. :-)

hypnoglow commented 8 years ago

Thank you, I have not thought of that solution with LESS environment variable - it is perfect. The PR may be closed now.

For a screen record have a look at this script. It records video, afterwards you can convert it to gif using gifsicle.

ymattw commented 8 years ago

Thanks for pointer to the tools!