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

Supported colored diff output #48

Open
haasn opened this issue Jan 24, 2022 · 5 comments · May be fixed by #54
Open

Supported colored diff output #48

haasn opened this issue Jan 24, 2022 · 5 comments · May be fixed by #54

Comments

@haasn
Copy link

haasn commented Jan 24, 2022

Coming from the Gentoo world I am really frustrated by how user-unfriendly rpmconf is in comparison to the fantastic dispatch-conf.

One very simple change that would make this tool a lot nicer to use would be the use of colored diff outputs (/usr/bin/diff --color=auto). I had a brief look at the code and it seems like you are using python's built-in difflib, which doesn't support this. So the fix would most likely be making rpmconf always use an external tool, and then let the user configure which one.

@xsuchy
Copy link
Owner

xsuchy commented Jan 30, 2023

@0x6d6e647a
Copy link

Fellow Gentoo user similarly annoyed by this. It would be nice if we could configure the diff tool via environmental variable or a command line flag to specify the diff tool used. I'd like to be able to use colordiff on CLI and kompare from the GUI.

@xsuchy I like the idea of using the Python diff library's coloring feature as well which would fit many people's needs. I think additionally providing a way to customize the diff program used would be nice as well. I can open a separate bug if you think what I'm saying is unrelated. If you don't mind pull requests I think I could implement this.

@xsuchy
Copy link
Owner

xsuchy commented Jul 19, 2023

It would be nice if we could configure the diff tool via environmental variable or a command line flag to specify the diff tool used.

You already can. Both on the command line and using env variable. See man rpmconf:

       -f<type>, --frontend=<type>
              Define which frontend should be used for merging. Valid options are: vimdiff, gvimdiff, diffuse, kdiff3, meld, sd‐
              iff and env. When set to env, the command to use is taken from the environment variable $MERGE. The default is env.

It is "just" matter of defining the TYPE. The code is here
https://github.com/xsuchy/rpmconf/blob/main/rpmconf/rpmconf.py#L294

PR is welcomed.

@0x6d6e647a
Copy link

In this branch, I have implemented, more or less, what @haasn and I were asking for. This was done by:

  • Adding a '--diff-frontend' CLI flag that specifies which tool is used to show the diff preview.
    • This supports all of the original tools that '--frontend' does plus diff, colordiff, and kompare.
    • This always creates a copy of the files before launching the tool because many of the tools and no way to enforce read only or check exit codes to see what happened.
  • Added CLI '--merge-frontend' as a synonym to '--frontend' to had consistent flag names without breaking CLI API.
  • Added support for kompare as a merge tool.
  • Updated documentation to reflect these changes.

I didn't see any unit tests in the repo so I didn't implement any for these changes. If that's incorrect please let me know and I'll write tests. If these changes are okay with you I'll open a pull request for this branch. If you have any comments / suggestions / ideas please feel free to let me know.

In this branch, I tried implementing color using Python's difflib similar to the link provided in this comment. It "works" except the pydoc pager refuses to interpret the color escape codes. When manually printing the same data and piping it to less -R it works but for some reason it does not work when using pydoc.pipepager. If you replace the call to pipepager with a simple print the escape color codes work fine. I'm not sure how to resolve this problem and was wondering if you maybe had some idea? I don't think replacing the pager with a direct print is a good solution.

@xsuchy
Copy link
Owner

xsuchy commented Jul 24, 2023

Can you file a pull request? That will be easier for review.

@0x6d6e647a 0x6d6e647a linked a pull request Jul 24, 2023 that will close this issue
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 a pull request may close this issue.

3 participants