Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fall back to plain diff when process substitution is used #978

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

th1000s
Copy link
Collaborator

@th1000s th1000s commented Feb 21, 2022

When detecting input generated by delta <(echo foo) <(echo bar)
fall back to plain diff instead of git diff --no-index.

This does not respect various git settings anymore (the original reason
to switch from diff to git diff), but is better than just showing
the names of the temporary files.

When detecting input generated by `delta <(echo foo) <(echo bar)`
fall back to plain `diff` instead of `git diff --no-index`.

This does not respect various git settings anymore (the original reason
to switch from `diff` to `git diff`), but is better than just showing
the names of the temporary files.
@dandavison
Copy link
Owner

Hi Thomas, do you think there's a viable alternative involving working with the file descriptor(s) and/or creating temporary files, along the lines of the discussion here: #929 (comment) ?

If possible I'd really like to keep the behavior consistent and use git diff

@th1000s
Copy link
Collaborator Author

th1000s commented Feb 23, 2022

That approach looks great and all the caveats I could think of are already being discussed, I like the piping-to-stdin so only one tmp-file has to be created.

But in the meantime, isn't perfect the enemy of good (enough)? :)

@dandavison
Copy link
Owner

But in the meantime, isn't perfect the enemy of good (enough)? :)

Yes, you're right! I just wanted to check that you're on board with the other solution. (I'm not aware that it's being actively worked on.)

@dandavison dandavison merged commit f165f56 into dandavison:master Feb 24, 2022
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.

None yet

2 participants