Add CLI --dry-run option for merge by louib · Pull Request #3254 · keepassxreboot/keepassxc

louib · 2019-06-10T21:19:42Z

@droidmonkey This PR is rebased on top of #3210.
This shows how options can be added to CLI commands once the cleanup is merged.

Type of change

✅ Refactor
✅ New feature
✅ Documentation

Description and Context

Two changes in fact: I added the --dry-run option and the list of changes detected by the merge operation to the output. In the future, I'd also like to ask the user for confirmation before merging, after displaying the changes.

Screenshots

$ keepassxc-cli merge -s --dry-run ~/test1.kdbx ~/test2.kdbx 
Insert password to unlock /home/louib/test1.kdbx: 
	Creating missing entry2 [06009d6280a741f88c4da6e4415d0aa5]
Database was not modified by merge operation.
$ keepassxc-cli merge -s ~/test1.kdbx ~/test2.kdbx 
Insert password to unlock /home/louib/test1.kdbx: 
	Creating missing entry2 [06009d6280a741f88c4da6e4415d0aa5]
Successfully merged the database files.
$ keepassxc-cli merge -s ~/test1.kdbx ~/test2.kdbx 
Insert password to unlock /home/louib/test1.kdbx: 
Database was not modified by merge operation.
$ keepassxc-cli merge -s --dry-run ~/test1.kdbx ~/test2.kdbx 
Insert password to unlock /home/louib/test1.kdbx: 
Database was not modified by merge operation.

Testing strategy

unit test + CLI (see screenshot)

Checklist:

✅ I have read the CONTRIBUTING document. [REQUIRED]
✅ My code follows the code style of this project. [REQUIRED]
✅ All new and existing tests passed. [REQUIRED]
✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]
✅ My change requires a change to the documentation, and I have updated it accordingly.
✅ I have added tests to cover my changes.

droidmonkey · 2019-06-10T23:02:18Z

Fantastic emoji use!

louib · 2019-06-11T00:24:15Z

😊

droidmonkey

Was it intentional to also include the changes from #3210? Seems like these got merged (no pun intended) 🙊

src/cli/Merge.cpp

louib · 2019-06-11T14:23:36Z

@droidmonkey yes, it's intended. Once #3210 is merged I'll rebase this one on top of develop. It's easier to add options to the CLI with the refactoring done in #3210, that's why I rebased the current PR on top of it.

droidmonkey · 2019-06-14T22:16:11Z

You can rebase on develop now. Also the other CLI PR's should be rebased and use the new command infrastructure.

louib · 2019-06-15T15:21:24Z

@droidmonkey rebased! I'll take care of the other CLI PRs later today. Thanks for the review!

src/core/Merger.h

comment was addressed

Added - Add 'Paper Backup' aka 'Export to HTML file' to the 'Database' menu [[#3277](#3277)] - Add statistics panel with information about the database (number of entries, number of unique passwords, etc.) to the Database Settings dialog [[#2034](#2034)] - Add offline user manual accessible via the 'Help' menu [[#3274](#3274)] - Add support for importing 1Password OpVault files [[#2292](#2292)] - Implement Freedesktop.org secret storage DBus protocol so that KeePassXC can be used as a vault service by libsecret [[#2726](#2726)] - Add support for OnlyKey as an alternative to YubiKeys (requires yubikey-personalization >= 1.20.0) [[#3352](#3352)] - Add group sorting feature [[#3282](#3282)] - Add feature to download favicons for all entries at once [[#3169](#3169)] - Add word case option to passphrase generator [[#3172](#3172)] - Add support for RFC6238-compliant TOTP hashes [[#2972](#2972)] - Add UNIX man page for main program [[#3665](#3665)] - Add 'Monospaced font' option to the notes field [[#3321](#3321)] - Add support for key files in auto open [[#3504](#3504)] - Add search field for filtering entries in Auto-Type dialog [[#2955](#2955)] - Complete usernames based on known usernames from other entries [[#3300](#3300)] - Parse hyperlinks in the notes field of the entry preview pane [[#3596](#3596)] - Allow abbreviation of field names in entry search [[#3440](#3440)] - Allow setting group icons recursively [[#3273](#3273)] - Add copy context menu for username and password in Auto-Type dialog [[#3038](#3038)] - Drop to background after copying a password to the clipboard [[#3253](#3253)] - Add 'Lock databases' entry to tray icon menu [[#2896](#2896)] - Add option to minimize window after unlocking [[#3439](#3439)] - Add option to minimize window after opening a URL [[#3302](#3302)] - Request accessibility permissions for Auto-Type on macOS [[#3624](#3624)] - Browser: Add initial support for multiple URLs [[#3558](#3558)] - Browser: Add entry-specific browser integration settings [[#3444](#3444)] - CLI: Add offline HIBP checker (requires a downloaded HIBP dump) [[#2707](#2707)] - CLI: Add 'flatten' option to the 'ls' command [[#3276](#3276)] - CLI: Add password generation options to `Add` and `Edit` commands [[#3275](#3275)] - CLI: Add XML import [[#3572](#3572)] - CLI: Add CSV export to the 'export' command [[#3278](#3278)] - CLI: Add `-y --yubikey` option for YubiKey [[#3416](#3416)] - CLI: Add `--dry-run` option for merging databases [[#3254](#3254)] - CLI: Add group commands (mv, mkdir and rmdir) [[#3313](#3313)]. - CLI: Add interactive shell mode command `open` [[#3224](#3224)] Changed - Redesign database unlock dialog [ [#3287](#3287)] - Rework the entry preview panel [ [#3306](#3306)] - Move notes to General tab on Group Preview Panel [[#3336](#3336)] - Enable entry actions when editing an entry and cleanup entry context menu [[#3641](#3641)] - Improve detection of external database changes [[#2389](#2389)] - Warn if user is trying to use a KDBX file as a key file [[#3625](#3625)] - Add option to disable KeePassHTTP settings migrations prompt [[#3349](#3349), [#3344](#3344)] - Re-enabled Wayland support (no Auto-Type yet) [[#3520](#3520), [#3341](#3341)] - Add icon to 'Toggle Window' action in tray icon menu [[3244](#3244)] - Merge custom data between databases only when necessary [[#3475](#3475)] - Improve various file-handling related issues when picking files using the system's file dialog [[#3473](#3473)] - Add 'New Entry' context menu when no entries are selected [[#3671](#3671)] - Reduce default Argon2 settings from 128 MiB and one thread per CPU core to 64 MiB and two threads to account for lower-spec mobile hardware [ [#3672](#3672)] - Browser: Remove unused 'Remember' checkbox for HTTP Basic Auth [[#3371](#3371)] - Browser: Show database name when pairing with a new browser [[#3638](#3638)] - Browser: Show URL in allow access dialog [[#3639](#3639)] - CLI: The password length option `-l` for the CLI commands `Add` and `Edit` is now `-L` [[#3275](#3275)] - CLI: The `-u` shorthand for the `--upper` password generation option has been renamed to `-U` [[#3275](#3275)] - CLI: Rename command `extract` to `export`. [[#3277](#3277)] Fixed - Improve accessibility for assistive technologies [[#3409](#3409)] - Correctly unlock all databases if `--pw-stdin` is provided [[#2916](#2916)] - Fix password generator issues with special characters [[#3303](#3303)] - Fix KeePassXC interrupting shutdown procedure [[#3666](#3666)] - Fix password visibility toggle button state on unlock dialog [[#3312](#3312)] - Fix potential data loss if database is reloaded while user is editing an entry [[#3656](#3656)] - Fix hard-coded background color in search help popup [[#3001](#3001)] - Fix font choice for password preview [[#3425](#3425)] - Fix handling of read-only files when autosave is enabled [[#3408](#3408)] - Handle symlinks correctly when atomic saves are disabled [[#3463](#3463)] - Enable HighDPI icon scaling on Linux [[#3332](#3332)] - Make Auto-Type on macOS more robust and remove old Carbon API calls [[#3634](#3634), [[#3347](#3347))] - Hide Share tab if KeePassXC is compiled without KeeShare support and other minor KeeShare improvements [[#3654](#3654), [[#3291](#3291), [#3029](#3029), [#3031](#3031), [#3236](#3236)] - Correctly bring window to the front when clicking tray icon on macOS [[#3576](#3576)] - Correct application shortcut created by MSI Installer on Windows [[#3296](#3296)] - Fix crash when removing custom data [[#3508](#3508)] - Fix placeholder resolution in URLs [[#3281](#3281)] - Fix various inconsistencies and platform-dependent compilation bugs [[#3664](#3664), [#3662](#3662), [#3660](#3660), [#3655](#3655), [#3649](#3649), [#3417](#3417), [#3357](#3357), [#3319](#3319), [#3318](#3318), [#3304](#3304)] - Browser: Fix potential leaking of entries through the browser integration API if multiple databases are opened [[#3480](#3480)] - Browser: Fix password entropy calculation [[#3107](#3107)] - Browser: Fix Windows registry settings for portable installation [[#3603](#3603)]

louib added the PR: In Progress label Jun 10, 2019

louib changed the title ~~Merge dry run~~ 🏜️🏜️ Adding --dry-run option for merge. Jun 10, 2019

louib mentioned this pull request Jun 10, 2019

Adding --dry-run option for merge. #2917

Closed

louib force-pushed the merge_dry_run branch 3 times, most recently from 0eca49a to 39672b6 Compare June 10, 2019 21:55

louib requested a review from droidmonkey June 11, 2019 00:24

droidmonkey requested changes Jun 11, 2019

View reviewed changes

src/cli/Merge.cpp Outdated Show resolved Hide resolved

droidmonkey added this to the v2.5.0 milestone Jun 11, 2019

louib force-pushed the merge_dry_run branch 2 times, most recently from 35f0fec to eef0c29 Compare June 11, 2019 14:47

louib requested a review from droidmonkey June 13, 2019 20:54

louib force-pushed the merge_dry_run branch from eef0c29 to fa075a0 Compare June 15, 2019 15:19

louib added feature: CLI and removed PR: In Progress labels Jun 15, 2019

droidmonkey previously requested changes Jun 15, 2019

View reviewed changes

src/core/Merger.h Show resolved Hide resolved

louib force-pushed the merge_dry_run branch from fa075a0 to 7635ac6 Compare June 16, 2019 23:52

louib requested a review from droidmonkey June 17, 2019 01:20

louib added 2 commits June 18, 2019 20:52

🏜️🏜️ Adding --dry-run option for merge.

84033db

📢 More explicit merge success message 📢

1108e9b

droidmonkey force-pushed the merge_dry_run branch from 7635ac6 to 1108e9b Compare June 19, 2019 00:55

droidmonkey approved these changes Jun 19, 2019

View reviewed changes

droidmonkey changed the title ~~🏜️🏜️ Adding --dry-run option for merge.~~ Add CLI --dry-run option for merge Jun 19, 2019

droidmonkey merged commit 84eec03 into keepassxreboot:develop Jun 19, 2019

louib deleted the merge_dry_run branch June 19, 2019 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add CLI --dry-run option for merge#3254

Add CLI --dry-run option for merge#3254
droidmonkey merged 2 commits intokeepassxreboot:developfrom
louib:merge_dry_run

louib commented Jun 10, 2019

Uh oh!

droidmonkey commented Jun 10, 2019

Uh oh!

louib commented Jun 11, 2019

Uh oh!

droidmonkey left a comment

Uh oh!

Uh oh!

louib commented Jun 11, 2019

Uh oh!

droidmonkey commented Jun 14, 2019

Uh oh!

louib commented Jun 15, 2019

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

louib commented Jun 10, 2019

Type of change

Description and Context

Screenshots

Testing strategy

Checklist:

Uh oh!

droidmonkey commented Jun 10, 2019

Uh oh!

louib commented Jun 11, 2019

Uh oh!

droidmonkey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

louib commented Jun 11, 2019

Uh oh!

droidmonkey commented Jun 14, 2019

Uh oh!

louib commented Jun 15, 2019

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants