Redmine

Is there any update on that? The patch definitely needs update as it doesn't apply cleanly; although, it works! I might be able to update the patch if more people are interested. I find it very useful and I 'd really like that to be merged in core...

By the way, how come you used ';' for value separator and not ',' (that is the default when adding multiple values via Redmine UI)?

Due to the changes done in r14503, users can be specified in different ways (name, email, username...) and the proposed path is not compatible with that. I understand that this patch more or less handles values that contain the separator but it would be much simplier to split the keyword first with the separator then search for the corresponding values.
Is it an acceptable solution?

Yiannis Tsiouris wrote:

By the way, how come you used ';' for value separator and not ','?

Because you're less likely to use ; in a possible value for a field than ,. For example, is "Simon, Garfunkel, Pitt, Brad" the values "Simon, Garfunkel" and "Pitt, Brad" or "Simon", "Garfunkel" and "Pitt, Brad"? It could also be "Simon" and "Garfunkel, Brad, Pitt" and so on.

; is still a compromise in that case. We could also use for example | (pipe symbol) or an even more obscure unicode character that has even less likelihood of being in a custom value, but then it gets tedious entering that in emails.

(that is the default when adding multiple values via Redmine UI)

Do you have an example? Do you mean when they are shown or when you enter them?

Jean-Philippe Lang wrote:

Due to the changes done in r14503, users can be specified in different ways (name, email, username...) and the proposed path is not compatible with that. I understand that this patch more or less handles values that contain the separator but it would be much simplier to split the keyword first with the separator then search for the corresponding values.
Is it an acceptable solution?

Splitting by separator first essentially makes all custom values containing that separator unfit to use in the email updates. The code would be simpler in that case though, yes.

If it is an explicit decision to say that such values with the separator will not work in the email keywords, it is fine for me. In this case using a less likely character as the separator, i.e. ; rather than , , would ensure that this edge case happens less often.

On the other hand source:/trunk/app/models/principal.rb@15884#L152 uses nearly the same technique as our proposed patch, even if it's written a little differently. It tries to match the name against the list of logins or mails or "firstname lastname" or names. That method could be refactored a bit to 2 methods, one emitting the list of logins, mails, "firstname lastname", names with the corresponding user objects, the other trying to match the passed string to that list. The first method could then be used in the fashion of the possible values for the custom fields, thus allowing to first create a list of possible values and then match the values against that list. We could prepare a patch proposal if this sounds like a working solution for you.

Please note that our proposed solution (first create a list of possible values, try to match those possible values against the value in the email) allows for the separator to be in the custom field possible values, it still has a caveat. If a possible value is a combination of 2 other values with the separator in between, this method can not decide which it matches. For example if the separator is ; and the possible values are "one", "two" and "one; two", our proposed method would not be able to decide wether "one; two" as a keyword value in an email is the values "one" and "two" or the value "one; two" for the custom field.

I hope I was clear enough in my explanation. I can provide more code and/or examples if necessary.

Felix Schäfer wrote:

Please note that our proposed solution (first create a list of possible values, try to match those possible values against the value in the email) allows for the separator to be in the custom field possible values, it still has a caveat. If a possible value is a combination of 2 other values with the separator in between, this method can not decide which it matches. For example if the separator is ; and the possible values are "one", "two" and "one; two", our proposed method would not be able to decide wether "one; two" as a keyword value in an email is the values "one" and "two" or the value "one; two" for the custom field.

Understood, that's what I meant when saying "I understand that this patch more or less handles values that contain the separator".

Thank you for your work on this. From what I read and judging from the tests I think this should work great!