Issue 7914

Title
Trim char/text field
Priority
feature
Status
resolved
Superseder
Product with newline in name is lost (issue 9248)
Nosy list
ced, lordvan, nicoe, pokoli, reviewbot, roundup-bot, timitos, yangoon
Assigned to
ced
Keywords
review

Created on 2018-12-10.13:31:35 by ced, last changed 2 weeks ago by roundup-bot.

Messages

New changeset 9542c372258d by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/tryton-env/rev/9542c372258d
New changeset e19329d25a6f by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/trytond/rev/e19329d25a6f
New changeset 75e45071896f by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/tryton/rev/75e45071896f
New changeset 5418e6544383 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/sao/rev/5418e6544383
New changeset e236e6ca4f2e by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/proteus/rev/e236e6ca4f2e
New changeset c2437a65b036 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/web_user/rev/c2437a65b036
New changeset 7817e8c48d79 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/web_shop_shopify/rev/7817e8c48d79
New changeset 4991d18e04cc by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/web_shop/rev/4991d18e04cc
New changeset 27201d2a76ab by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/timesheet/rev/27201d2a76ab
New changeset 935edca4f028 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/stock_package_shipping_sendcloud/rev/935edca4f028
New changeset bb898653d808 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/stock_package_shipping_mygls/rev/bb898653d808
New changeset 8a7972c1e76e by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/stock_package_shipping_dpd/rev/8a7972c1e76e
New changeset f7693a143c69 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/stock_package_shipping/rev/f7693a143c69
New changeset 78fbe23795c1 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/product/rev/78fbe23795c1
New changeset 288942e9f597 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/party/rev/288942e9f597
New changeset 695f23391e69 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/marketing_automation/rev/695f23391e69
New changeset 9241b14e3383 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/currency_rs/rev/9241b14e3383
New changeset c3c7cf3e6ba5 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/currency/rev/c3c7cf3e6ba5
New changeset 2348d81854fc by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/account_payment_stripe/rev/2348d81854fc
New changeset 4a92175f38b0 by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/account_payment_braintree/rev/4a92175f38b0
New changeset efb5fe29e90d by Cédric Krier in branch 'default':
Add option to remove leading and trailing white spaces from char
https://hg.tryton.org/modules/account_fr_chorus/rev/efb5fe29e90d
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-09-10.00:31:43

I have added the possibility to set strip to leading and trailing. This way we can manage correctly field that are used as prefix or suffix (like template code and product suffix).

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2022-07-02.10:50:21

For me the Python str.strip should be as fast as the SQL version. Also in worst case, doing it in Python reduce the size of the query sent to the server.

Author: [hidden] (lordvan)
Date: 2022-01-04.19:52:52

One thing I just thougth about when reading this .. wouldn't it be more performant to do this in SQL instead of python itself? (this is just a guess on my part btw). - Also I do not know if any used/supported DB backends would have problems with this ?

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-04-02.19:29:14

Please assign the issue to yourself and set correct status, see https://www.tryton.org/develop#report-issue

Author: [hidden] (nicoe) Tryton committer
Date: 2021-04-02.19:17:55

Here is the review adding the strip option.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2020-10-24.17:26:23

This is partially solved by issue9248. But I still sting it is good to trim trailing spaces for most Char fields on client side.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2020-10-24.17:08:10

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).

Author: [hidden] (nicoe) Tryton committer
Date: 2018-12-12.10:12:31
* Cédric Krier [2018-12-11 18:26:17]:
> 
> Cédric Krier <cedric.krier@b2ck.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.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-12-11.18:26:17
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.
Author: [hidden] (yangoon) Tryton translator
Date: 2018-12-11.13:58:22
I absolutely agree with nicoe (msg45232).
Author: [hidden] (nicoe) Tryton committer
Date: 2018-12-10.16:38:34
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.
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2018-12-10.15:57:44
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).
Author: [hidden] (timitos) Tryton translator
Date: 2018-12-10.15:50:13
+1
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-12-10.15:47:16
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.
Author: [hidden] (timitos) Tryton translator
Date: 2018-12-10.15:40:12
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.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-12-10.15:30:05
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.
Author: [hidden] (timitos) Tryton translator
Date: 2018-12-10.13:45:57
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?
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-12-10.13:31:35
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.
History
Date User Action Args
2022-09-19 21:28:22roundup-botsetmessages: + msg78246
2022-09-19 21:28:17roundup-botsetmessages: + msg78245
2022-09-19 21:28:13roundup-botsetmessages: + msg78244
2022-09-19 21:28:10roundup-botsetmessages: + msg78243
2022-09-19 21:28:06roundup-botsetmessages: + msg78242
2022-09-19 21:28:02roundup-botsetmessages: + msg78241
2022-09-19 21:27:57roundup-botsetmessages: + msg78240
2022-09-19 21:27:54roundup-botsetmessages: + msg78239
2022-09-19 21:27:50roundup-botsetmessages: + msg78238
2022-09-19 21:27:46roundup-botsetmessages: + msg78237

Showing 10 items. Show all history (warning: this could be VERY long)