Showing all the differences
Lantizia opened this issue · 8 comments
Unless I've completely missed something (and it wouldn't be the first time), this program doesn't seem to actually show a full diff.
Say database1.kdbx and database2.kdbx are near-identical (perhaps the second database file only exists due to a sync conflict with a provider like Nextcloud) and database1.kdbx has an extra Entry (that database2.kdbx lacks entirely) called 'New Entry' in the Group 'My Group' and has fields like Username and Password populated.
At the moment keepass-diff (shown here with verbose turned on) only tells you...
~ Group 'Root'
~ Group 'My Group'
- Entry 'New Entry'
But it would be handy if it could actually tell me what was is in that entry...
~ Group 'Root'
~ Group 'My Group'
- Entry 'New Entry'
- Field 'Username' = 'an@email.address'
- Field 'Password' = 'password'
At the moment I have to open both copies of the database (KeePassXC in my case) and end up just using keepass-diff to just give me hints as to what I should be comparing between both of them (so more like... keepass-diff-hints 😃). Granted this does mean I can just drag entries from one database to another... but this is often overkill if it's just the odd password that has changed.
It'd be better (e.g. mostly for small differences between databases) to not need to open both databases and just have keepass-diff tell me what the actual differences were... so that I can update the canonical database myself manually.
I know there is currently an issue where attachments aren't shown in diffs (luckily I don't have any in my databases yet)... but I'm guessing this is meant to show every other attribute? Regardless of if they're custom, protected, or otherwise?
Hi @Lantizia, thanks for reaching out!
Indeed, the initial intent of keepass-diff was to show me which entries I need to look at. If an Entry is missing, it means the full entry needs to be copied over. If only parts of an Entry are different - let's say in your example the field Password was missing but Username was set, it should show something like this:
~ Group 'Root'
~ Group 'My Group'
~ Entry 'New Entry'
- Field 'Password' = 'password'
Do I understand it correctly, you suggest that we show all the Fields of a missing Entry as well, so you can directly see the missing values and are able to put them in the one where they are missing?
If that's what you mean, I'd propose a solution to add an additional flag for this feature - would you have a suggestion for a name for that? 😅
Indeed, you understand me correctly.
Maybe '--show-all-fields' ? Possibly '--show-entry-contents-if-missing' ? Sarcastically '--actually-diff' ?
I'm not good at naming things 😄
Personally I think it should be the other way around.
Take the actual diff command for example, which would normally show something like this...
lantizia@loki:~$ diff -c a b
*** a 2022-10-06 18:18:05.937077679 +0100
--- b 2022-10-06 18:18:15.493071443 +0100
***************
*** 1,4 ****
--- 1,8 ----
This is a line
and another line
Wow lines are fun
+ # Fun times will be had with hyphens
+ # - meow
+ # - moo
+ # - oink
Badgers
But instead diff did this... (extremely sarcastic example ahead...)
lantizia@loki:~$ diff -c a b
*** a 2022-10-06 18:18:05.937077679 +0100
--- b 2022-10-06 18:18:15.493071443 +0100
***************
*** 1,4 ****
--- 1,8 ----
This is a line
and another line
Wow lines are fun
+ # Fun times will be had with hyphens
! NOTE FROM DIFF: Some other lines were here, won't tell you how many or what they were... life eh?
Badgers
If you agree with that... then I'd argue someone needs to state '--brief' (or something like that) to get the behaviour how it currently works 😄
Just got tripped up by something so I'm adding to this issue as it applies to this situation too...
Basically two copies of the same database again, only small differences (due to Nextcloud syncing issues)...
- [Root, Some Org, Panel, Password = old pass]
+ [Root, Some Org, Panel, Password = new pass]
+ [Root, Lantizia, Thing]
- [Root, Lantizia, Thing, Notes = An old note]
+ [Root, Lantizia, Thing, Notes = An updated note]
- [Root, House, Nebula, URL = https://nebula.tv]
+ [Root, House, Nebula, URL = https://nebula.app]
So I look at this and I manually update the canonical copy of my database to fix the password for 'Panel', fix the note for 'Thing' and fix the URL for 'Nebula'.
Although I did find it strange (at the time) that it showed an additional line for 'Thing'. But after checking that the canonical copy of my database did in fact have an item called 'Thing' in the folder 'Lantizia'... I thought little of it and moved on.
Moved on... to the point of deleting that database... the database which (at the time) I thought I'd compared and seen anything of use out of it... integrating it into my main database.
Luckily Nextcloud keeps deleted files!
As what I should have realised is that within that database was an additional item called 'Thing'.
Basically two items called 'Thing' in the same directory. And the one I had missed was quite useful!
If however this actually showed a full diff... rather than just abbreviating sections... then I'd have spotted that the other 'Thing' item was for a completely different username and realised I needed that too.
Hi @Lantizia, sorry for not responding in the last few weeks, it's been a bit busy over here. I do understand the issue and I think you got a point that this full version should be supported.
I'm still undecided whether it should be the default behavior or behind a flag, to be honest. If anybody wants to weigh in with pro and contra arguments, I would be happy. Also a good name for the enable/disable flag this would be nice 😅
Maybe a config file feature would be an option? Something like git config? This could allow people to configure default behavior the way they want it to be.
Maybe a config file feature would be an option? Something like git config?
Is there a recommended module in Rust that allows this without this tool having to address all the different operating systems and issues around it? Something like dotenv in Node?
But to be honest, it sounds like overkill to me to add a config file for keepass-diff. This tool should only be necessary in rare circumstances. If you'd like to change the default, I suppose you could alias keepass-diff with the option set directly...
There is some crates.io, but this can be implemented using just std. But I guess the simpler solution would be as you said, just use shell aliases.
If anybody wants to weigh in with pro and contra arguments, I would be happy.
ok... i'll give it a go:
pro:
- considering the fact, we are talking about a diff-tool which is mostly used in a process of merging conflicting duplicates and deleting the remains, IMHO the approach should always be to rather show to much by default, then too little.
- additionally as we are talking about a CLI, it is save to say that it is rather used by advanced users than noobs... so a good portion of the actual users may already be used to other diff-tools which usually show all differences (at least by default)!
contra:
- it may seem a bit overwhelming if you start with big files with many changes...
well, first, this might be seldom the case and, second, as i mentioned the actual users usually know how to use a CLI and easily find the help/man page (with additional options).
speaking about the option... yes, i know it's not anyhow inventive but most CLI i know use -q (--quiet) or -v (--verbose) to get less or more output. it may not be completely self-explaining but a no-brainer to remember and also easily extendable (-qqq or -q3 resp. --quiet 3) if there are more levels of output in the future.
PS: at first i also wasn't sure if this is really needed... but thankfully i stumbled across this issue and hence was aware to be on the lookout. and after some more heavily usage of keepass-diff (even for files i don't use daily and thus don't know that good) i changed my mind and think it is absolutely necessary to have all differences shown (ideally by default)!
...i had a similar incident as #51 (comment) but with renamed similar entries. - thanks @Lantizia, this issue saved me a hell of troubles!