It would be nice to have this possibilty to use ldapmodify easily, but as far as I see the patch only covers a subset of what is possible with ldapmodify.

ldapmodify allows multiple values in one ADD/DEL/REPLACE operation, it also allows DEL/REPLACE without a value to completely remove an attribute.
I think the error handling based on the number of args shoul be done after determining the type and allow a list of values

So if I'm reading this, I think you are suggesting something like:

"add:attribute:value:value:value"? Is that what you mean here?

The del/replace with no value to remove an attribute however, is an oversight, so I can fix this to allow "delete:attr:" to remove an attribute completely.

@lkrispen What do you think of the above suggestion for the value list? :)

I will let @lkrispen comment but he may had in mind something like

dn: dc=foo19,dc=example,dc=com
changetype: modify
add: sn
sn: x3
sn: x4
-
delete: sn
sn: x1
-
replace: cn
cn: foo

Do you want the generic update to cover these kind of update ?

@tbordaz I'd ready like to avoid that syntax-style because it's basically "reinventing ldif", and it's a bit awkward. This tool is intended for quick single attribute or a few attribute changes, not large bulk updates like this. Part of the motivation is that asking people to provide a file, and then apply it, is a barrier to entry because the syntax of that file is foreign. But having a cli tool we get something easier and quicker to use, and then admins can "step up" to ldif if they require the complete ability for ldap to express complex changes like that.

Does this help explain a bit of my motivation?

@firstyear, thanks for clarification. Supporting only 'simple' updates looks fine to me as it is likely the vast majority of admins need.

Delete now allows deleting all values with "delete:attr:". I think that we can do the multiple value replace with "delete:attr:" "add:attr:value" "add:attr:value2" instead rather than writing a multi-value parser.

I haven't tested but the code looks good.

@spichugi Great thanks! I'll give @lkrispen another day to comment, and then I'll merge tomorrow if that's okay :)

@spichugi Great thanks! I'll give @lkrispen another day to comment, and then I'll merge tomorrow if that's okay :)

I'd like for @mreynolds to check it too and leave his opinion...

It's a new functionality (which I agree that we'll need someday) but we will have it in master after the merge. And the master is going to the soonest RHEL release.
To me, it looks like it makes more sense to branch it (or postpone the merge) for now and to write a design doc for the new syntax.

But maybe I am overthinking the thing and Mark will correct me.

P.S. sorry if it's too much of a resistance. I really like the thing but we should be careful about what we deliver.

I'm on the fence about when to merge this. It does make sense to wait for 1.4.1 branch (which I plan to do soon). @firstyear any objections waiting for 1.4.1?

@mreynolds Maybe this is a good time to test feature gating by version? We could have a flag for "if defaults. version >= 1.4.1" then to add the arg parse lines? This way we could merge now, and it would only unblock on the future version bump? I think we should consider this gating strategy now since we are running into it.

As @mreynolds has branched, I'll commit this now.

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3217

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.