Bug 143193 - Clean up Basic 'Format' implementation
Summary: Clean up Basic 'Format' implementation
Status: ASSIGNED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium enhancement
Assignee: Paris Oplopoios
URL:
Whiteboard:
Keywords: difficultyInteresting, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2021-07-05 06:17 UTC by Mike Kaganski
Modified: 2023-08-09 10:12 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2021-07-05 06:17:29 UTC
Format function [1] is implemented in SbxValue::Format [2]. It has two sections, first of which uses SvNumberFormatter, and the second uses home-grown SbxBasicFormater. The latter handles non-numeric input, such as strings, Nulls, etc., and also special Basic string formats like "Standard" or "ttttt". (Note that the first section also handles some special formats, like "<" or "General Date".)

Also the latter function handles potential numeric string cases which couldn't be converted to number using SvNumberFormatter::IsNumberFormat, but succeeded using ScanNumIntnl.

This makes the implementation buggy, inconsistent, and over-engineered. The task is to unify special format string handling in the beginning of the implementation, then process Null values, check if string should be converted to number (analyzing the type of format string using SvNumberFormatter facilities) and if it can be converted, and then use the normal SvNumberFormatter processing.

The implementation should drop the special processing of "@" format character used to indicate insertion of thousand separators (used currently when processing "Standard" format) and non-standard string formatting characters '!' and '\' (see bug 143183 comment 2).

The fix must include an extensive unit test.

[1] https://help.libreoffice.org/7.2/en-US/text/sbasic/shared/03120301.html?DbPAR=BASIC
[2] https://opengrok.libreoffice.org/xref/core/basic/source/sbx/sbxscan.cxx?r=0771ac00&mo=19326&fi=660#660
Comment 1 Paris Oplopoios 2022-03-28 11:45:59 UTC
(In reply to Mike Kaganski from comment #0)
> Format function [1] is implemented in SbxValue::Format [2]. It has two
> sections, first of which uses SvNumberFormatter, and the second uses
> home-grown SbxBasicFormater. The latter handles non-numeric input, such as
> strings, Nulls, etc., and also special Basic string formats like "Standard"
> or "ttttt". (Note that the first section also handles some special formats,
> like "<" or "General Date".)
> 
> Also the latter function handles potential numeric string cases which
> couldn't be converted to number using SvNumberFormatter::IsNumberFormat, but
> succeeded using ScanNumIntnl.
> 
> This makes the implementation buggy, inconsistent, and over-engineered. The
> task is to unify special format string handling in the beginning of the
> implementation, then process Null values, check if string should be
> converted to number (analyzing the type of format string using
> SvNumberFormatter facilities) and if it can be converted, and then use the
> normal SvNumberFormatter processing.
> 
> The implementation should drop the special processing of "@" format
> character used to indicate insertion of thousand separators (used currently
> when processing "Standard" format) and non-standard string formatting
> characters '!' and '\' (see bug 143183 comment 2).
> 
> The fix must include an extensive unit test.
> 
> [1]
> https://help.libreoffice.org/7.2/en-US/text/sbasic/shared/03120301.
> html?DbPAR=BASIC
> [2]
> https://opengrok.libreoffice.org/xref/core/basic/source/sbx/sbxscan.
> cxx?r=0771ac00&mo=19326&fi=660#660

VBA seems to treat "@" in a different way as you said in bug 143183 comment 2.

Should the Format function be changed to match that in order for this to be considered complete? I have done some work on this in https://gerrit.libreoffice.org/c/core/+/132137 but I don't feel like it's complete yet.
Comment 2 Mike Kaganski 2023-08-09 10:12:25 UTC
(In reply to Paris Oplopoios from comment #1)
> VBA seems to treat "@" in a different way as you said in bug 143183 comment
> 2.
> 
> Should the Format function be changed to match that in order for this to be
> considered complete? I have done some work on this in
> https://gerrit.libreoffice.org/c/core/+/132137 but I don't feel like it's
> complete yet.

Sorry for the late reply (missed this somehow); and sorry for not understanding the question: what specifically do you mean?