[Patch] Use multi-tag separator from KeywordSuggestPlugin when installed

patch against tagsplugin trunk

Ideally the option should reside in the tags configuration section, as this would be the place you'd look for it intuitively.

But keywordsuggest has been first in using it, and I was thinking exactly the same when looking at comment 20 in #9061.

Well, I guess if we're going for the ideal, the option should reside in the ticket configuration section, and be used also by Trac core for the keywords field.

The path of minimal conflict is the one with least backward-incompatible changes, which is what I suggested. It does not introduce a dependency on KeywordSuggestPlugin per se, since it has default value if the option is not present.

Replying to itamarost:

Well, I guess if we're going for the ideal, the option should reside in the ticket configuration section, and be used also by Trac core for the keywords field.

Agree. There should be a common way of splitting field information. Use the one in Trac or a minimal compat handler in plugin to support older versions if needed. It could be general Trac though, and not just tickets. I could use the same logic for splitting categories in FullBlogPlugin posts.

Personally I think it should be a bit more elaborate than a character in a config, be it whitespace or comma or combination thereof. It should use some heuristics to determine the the current practice for the field and reuse that for splitting and joining.

Ideally it should be managed without settings by just being 'intuitive', and doing the 'right thing™'. I would help get such a patch with split/join utility functions committed to Trac.

Replying to osimons:

Ideally it should be managed without settings by just being 'intuitive', and doing the 'right thing™'.

The reason being of course that a hidden setting is very confusing when working with different Trac projects - you should not need to know a config setting to edit fields correctly.

Replying to osimons:

Replying to osimons:

Ideally it should be managed without settings by just being 'intuitive', and doing the 'right thing™'.

The reason being of course that a hidden setting is very confusing when working with different Trac projects - you should not need to know a config setting to edit fields correctly.

I second the emphasis on usability, but do you suggest to operate like this on a single value base? Using different delimiter styles within one table column would be a real pain for any SQL query with multiple lines involved, if reasonable at all within certain performance constraints.

Well, SQL on multi-value fields is always going to be a pain no matter what. However, I do see your point and it would help if there was some normalization for storage (which of course none of the fields do today). My meaning was more that the we should accept common patterns and not settle on a single character.

Here is my current favorite:

>>> import re
>>> s = 'tag1, tag2,tag3 tag4, "tag 5", "tag,6", "tag\'7"'
>>> [p.strip(", \"'") for p in re.split("(,| |\\\".*?\\\"|'.*?')", s) if p.strip(", \"'")]
['tag1', 'tag2', 'tag3', 'tag4', 'tag 5', 'tag,6', "tag'7"]

It isn't easy this, but we should try to not confuse things even further.

Replying to osimons:

Well, SQL on multi-value fields is always going to be a pain no matter what.

I see. For tickets there's no hope to change this, before multiple values per field (see t:#918) get implemented in general, right? Maybe some split and merge logic is not perfect yet, but TagsPlugin itself can store multiple tags per resource in tags Trac db table, so there is no such limitation for tags in general.

However, I do see your point and it would help if there was some normalization for storage (which of course none of the fields do today).

In fact KeywordSuggestPlugin already did some sanitization in the past, and this is another thing I like about it very much. But indeed, nothing in Trac core. Nevertheless, we should reach for better standards in this respect. I mean, for date/time fields it's natural, to parse and accept only valid entries, why not for keywords/tags too?

My meaning was more that the we should accept common patterns and not settle on a single character. Here is my current favorite: ... It isn't easy this, but we should try to not confuse things even further.

I'm all for accepting every string and trying to pull out valid tag(s) with best available logic. As I see it, this is not the end of the comma as a delimiter in general, just for storage. In the same move we could (should?) resort to reformatting according to current delimiter settings just for display.

At least this is my understanding of your proposal. Do we already have an agreement on this? More comments welcome, especially regarding display and normalization of stored values.

#9061 suggests possibly unwanted tag list normalization by the new tag mass edit functionality. Just something to consider when preparing the solution here.

Yet another case, another objective to consider:

Oblio Leitch: Well, in my [use] case, I'm applying the plugin to a very active ticket system - there are already thousands of tickets. When I apply TagPlugin, the cloud that's created has lots of useless tags being generated by the keywords field in the existing tickets. Tags like "-", "&", "7", "I", etc. Yes, the keywords field shouldn't be used as a description field (eg "Defect: P & I Calc Date (Dormant)"), however, going through and changing all the keywords would be time consuming. It would be helpful to simply change the delimiter to a comma.

citation from trac-users mailing-list