xsuchy / rpmconf

Tool to handle rpmnew and rpmsave files
43 stars 19 forks source link

Supported colored diff output #48

Open haasn opened 2 years ago

haasn commented 2 years ago

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 commented 1 year ago

Or we can use something like https://stackoverflow.com/questions/32500167/how-to-show-diff-of-two-string-sequences-in-colors

0x6d6e647a commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

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 commented 1 year ago

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