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

Add a -n (--notes) option to keepassxc-cli add and edit commands #4646

Merged

Conversation

grimkor
Copy link

@grimkor grimkor commented Apr 22, 2020

Was working on exporting my data from Bitwarden where I use notes to KeePass and noticed the keepassxc-cli was missing notes as an option for adding and editing records. Looked easy enough to plug in so figured I would give it a go. Hopefully it is to taste!

Screenshots

edit_add
help
man

Testing strategy

Manual testing performed with existing and newly created databases through the develop branch version. Changes show to be reflected correctly in the databases.

Type of change

  • ✅ New feature (change that adds functionality)

@droidmonkey
Copy link
Member

droidmonkey commented Apr 22, 2020

Technically you can do this by specifying an attribute of name "Notes" from the command line. I do like the shortcut though!

@grimkor
Copy link
Author

grimkor commented Apr 22, 2020

Technically you can do this by specifying an attribute of name "Notes" from the command line. I do like the shortcut though!

Oh no! Did I do this as a result of failing to read the manual properly?

@louib
Copy link
Member

louib commented Apr 22, 2020

@droidmonkey @grimkor I feel like it makes sense to have a standalone option for that, since the notes are part of the primary attributes set.

Since the notes are a multilines text field, should we split the value on \n?

Also, some new unit tests for the option would be nice :)

@droidmonkey
Copy link
Member

Possibly? It's certainly not intuitive unless you know how kdbx works.

@copra7070
Copy link

Thanks

@grimkor
Copy link
Author

grimkor commented Apr 22, 2020

@droidmonkey @grimkor I feel like it makes sense to have a standalone option for that, since the notes are part of the primary attributes set.

Since the notes are a multilines text field, should we split the value on \n?

Also, some new unit tests for the option would be nice :)

Sure I can take a look at splitting the values based on \n c++ is a bit new to me so it might take longer than a more seasoned person.

With regards to the attributes that @droidmonkey mentioned I went back to take a look at the add & edit but couldn't see where you can specify additional attributes in those.

@droidmonkey
Copy link
Member

You're right, we also need a way to add arbitrary attributes to entries.

@grimkor grimkor force-pushed the feature/add-notes-from-cli branch 3 times, most recently from 6404259 to efa11d2 Compare April 23, 2020 11:18
@copra7070
Copy link

Thanks

@droidmonkey droidmonkey added this to the v2.6.1 milestone May 21, 2020
@grimkor grimkor force-pushed the feature/add-notes-from-cli branch from 0fded03 to c733f77 Compare June 22, 2020 14:50
@grimkor
Copy link
Author

grimkor commented Jun 22, 2020

I'll be honest guys I'm way out of my depth here. There was a merge conflict and I thought I had it but I don't. The errors I can see from TeamCity are outside of my scope of comfort and I don't really want to keep making changes without understanding the language and the project.

Not sure what you would want done here but I am happy to assist where possible.

@droidmonkey droidmonkey modified the milestones: v2.6.1, v2.7.0 Jul 23, 2020
@louib
Copy link
Member

louib commented Sep 19, 2020

@grimkor @droidmonkey I feel like this one is almost ready to merge. We'd just have to make sure it's correctly rebased with develop and validate that the line breaks behavior is what we want. Currently we need escaping when setting the notes (i.e. -n "testing\\nline breaks")

@droidmonkey
Copy link
Member

Agree

@droidmonkey droidmonkey force-pushed the feature/add-notes-from-cli branch from c733f77 to ee38ca9 Compare September 21, 2020 02:49
@droidmonkey
Copy link
Member

Rebased and ready for merge. I cleaned up the code a bit too.

@droidmonkey droidmonkey merged commit 55eb855 into keepassxreboot:develop Sep 21, 2020
@phoerious phoerious added pr: new feature Pull request that adds a new feature and removed new feature labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: CLI pr: new feature Pull request that adds a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants