Created on 2018-12-10.13:31:35 by ced, last changed 1 month ago by reviewbot.
Please assign the issue to yourself and set correct status, see https://www.tryton.org/develop#report-issue
Here is the review adding the strip option.
This is partially solved by issue9248. But I still sting it is good to trim trailing spaces for most Char fields on client side.
I think you're confusing the value and the way the value is displayed.
But as the trim will happen at the UI, for me it is the same behavior.
Yet I don't think it's a good idea because it breaks the principle of least surprise
It will not be a surprise if it happens on the UI so the user will see it (but not sure because we remove invisible chars).
* Cédric Krier [2018-12-11 18:26:17]: > > Cédric Krier <email@example.com> added the comment: > > Indeed we already do such transformation on some fields. E.g. we > remove the non-significant zero (and trim space) on numeric; we > transform text into int for Many2One, we recompute sequence field. I think you're confusing the value and the way the value is displayed. In case of char data they're both the same, while it's not the case when the data is a relation, a number or an order relationship. > In many char/text fields, trailing spaces have no signification for > the user. Usually it's the case but sometimes it might not be it. > I would not activate on Party.name because it is too much special > but I think it should on almost all other names, If party.name is an exception then trim should default to True for fields.Char. Yet I don't think it's a good idea because it breaks the principle of least surprise: users and developers expect that the data they filled will be saved as-is. > codes, numbers and description. I agree that it can be useful for those fields though.
Indeed we already do such transformation on some fields. E.g. we remove the non-significant zero (and trim space) on numeric; we transform text into int for Many2One, we recompute sequence field. In many char/text fields, trailing spaces have no signification for the user. I would not activate on Party.name because it is too much special but I think it should on almost all other names, codes, numbers and description.
I absolutely agree with nicoe (msg45232).
It's a bit strange to consider that you can fiddle with user input but I can understand that for codes or other identifiers it makes sense to trim the data (according to me those fields shouldn't usually be editable in the interface anyway). Making it an opt-in feature is OK but we should use it parsimoniously.
In sao line breaks are also not shown so I agree that it will be better if we can remove it. Using str.strip sounds like the more reasonable solution for this case. I agree that it should be enabled for name's but I'm wondering if it makes sense to activate it also for codes (i.e: Product code, account code, etc) and also for description (i.e: product description, sale line description, invoice line description).
If we use str.strip, it will remove any line breaks at the end or beginning but not inside. I think it is a good behavior.
They are not visible when the client is used on windows for example. So if you use tryton in a environment with client on different os, for example a combination of linux and windows the line breaks are only visible on the linux machines but not on the windows machines.
On 2018-12-10 13:45, Korbinian Preisler wrote: > There is another point that could be considered with this. We sometimes have to deal with line separators at the end of the string, especially when pasting from ms excel or libreoffice calc. Could this feature be extended to deal with things like that too? Those line breaks are visible in GTK entry (I do not know for HTML input) So if the user can see and correct them, I do not think we should overrule their usage.
There is another point that could be considered with this. We sometimes have to deal with line separators at the end of the string, especially when pasting from ms excel or libreoffice calc. Could this feature be extended to deal with things like that too?
I saw this: https://github.com/odoo/odoo/commit/fb6c9c24434c941383ffd73ab3056cafd7740c52 I think it is not a bad idea to have such behavior as I saw many times trailing spaces on names (like product, party etc.). I think it will be wrong to manage it only on client side (like Odoo) but it should also be enforced on server side. Because it will be inconsistent is trailing spaces are added via some RPC calls and changed on the fly by the client. Also I think it should be an opt-in feature not an opt-out. For text it should trim each line.
|2021-06-15 19:06:35||reviewbot||set||messages: + msg68275|
|2021-06-10 13:00:44||reviewbot||set||messages: + msg68184|
|2021-05-19 15:35:10||lordvan||set||nosy: + lordvan|
|2021-05-11 19:42:04||reviewbot||set||messages: + msg67520|
|2021-05-11 18:21:51||nicoe||set||assignedto: nicoe|
|2021-05-11 18:21:42||nicoe||set||status: chatting -> in-progress|
|2021-04-02 19:29:14||ced||set||messages: + msg66008|
nosy: + reviewbot
messages: + msg66006
superseder: + Product with newline in name is lost
|2020-10-24 17:08:11||ced||set||messages: + msg61362|
|2018-12-12 10:12:32||nicoe||set||messages: + msg45285|
|2018-12-11 18:26:17||ced||set||messages: + msg45266|
|2018-12-11 15:07:59||ced||set||component: + tryton, trytond, sao|
messages: + msg45261
messages: + msg45232
messages: + msg45231
|2018-12-10 15:50:14||Timitos||set||messages: + msg45230|
|2018-12-10 15:47:17||ced||set||messages: + msg45229|
|2018-12-10 15:40:12||Timitos||set||messages: + msg45228|
|2018-12-10 15:30:05||ced||set||messages: + msg45227|
|2018-12-10 13:45:57||Timitos||set||status: unread -> chatting|
nosy: + Timitos
messages: + msg45226