Skip to content

Fix AsYouTypeFormatter dropping parens from rule-based formats (BR, RU, CO) #3993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PetroczyP wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix AsYouTypeFormatter dropping parens from rule-based formats (BR, RU, CO) #3993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 51 additions & 3 deletions cpp/src/phonenumbers/asyoutypeformatter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ const char kSeparatorBeforeNationalNumber = ' ';
// number when formatting.
const char kNationalPrefixSeparatorsPattern[] = "[- ]";

// Matches the first $N group reference in a format string. Local copy of the
// regex held by PhoneNumberUtil::PhoneNumberRegExpsAndMappings::
// first_group_capturing_pattern_; duplicated to avoid widening that field's
// visibility.
const char kFirstGroupPattern[] = "(\\$\\d)";

// Matches all the groups contained in 'input' against 'pattern'.
void MatchAllGroups(const string& pattern,
const string& input,
Expand Down Expand Up @@ -239,7 +245,8 @@ void AsYouTypeFormatter::SetShouldAddSpaceAfterNationalPrefix(

bool AsYouTypeFormatter::CreateFormattingTemplate(const NumberFormat& format) {
string number_pattern = format.pattern();
string number_format = format.format();
string number_format;
ApplyNationalPrefixFormattingRule(format, &number_format);
formatting_template_.remove();
UnicodeString temp_template;
GetFormattingTemplate(number_pattern, number_format, &temp_template);
Expand All @@ -251,6 +258,39 @@ bool AsYouTypeFormatter::CreateFormattingTemplate(const NumberFormat& format) {
return false;
}

// See header for the three-branch contract and the BuildMetadataFromXml
// substitution invariant; this implementation just mirrors that prose.
void AsYouTypeFormatter::ApplyNationalPrefixFormattingRule(
const NumberFormat& format, string* number_format) const {
DCHECK(number_format);
number_format->assign(format.format());
string rule = format.national_prefix_formatting_rule();
bool is_international_mode =
is_complete_number_ && extracted_national_prefix_.empty();
if (rule.empty() || is_international_mode) {
return;
}
const string& national_prefix = current_metadata_->national_prefix();
bool stripped_prefix = false;
if (!national_prefix.empty() &&
rule.compare(0, national_prefix.size(), national_prefix) == 0) {
rule = rule.substr(national_prefix.size());
if (!rule.empty() && (rule[0] == ' ' || rule[0] == '-')) {
rule = rule.substr(1);
}
stripped_prefix = true;
}
if (!stripped_prefix && !national_prefix.empty() &&
rule.find(national_prefix) != string::npos) {
return;
}
static const scoped_ptr<const RegExp> first_group_pattern(
regexp_factory_->CreateRegExp(kFirstGroupPattern));
bool status = first_group_pattern->Replace(number_format, rule);
DCHECK(status);
IGNORE_UNUSED(status);
}

void AsYouTypeFormatter::GetFormattingTemplate(
const string& number_pattern,
const string& number_format,
Expand Down Expand Up @@ -477,8 +517,10 @@ void AsYouTypeFormatter::AttemptToFormatAccruedDigits(
SetShouldAddSpaceAfterNationalPrefix(number_format);

string formatted_number(national_number_);
string effective_format;
ApplyNationalPrefixFormattingRule(number_format, &effective_format);
bool status = regexp_cache_.GetRegExp(pattern).GlobalReplace(
&formatted_number, number_format.format());
&formatted_number, effective_format);
DCHECK(status);
IGNORE_UNUSED(status);

Expand Down Expand Up @@ -687,8 +729,14 @@ bool AsYouTypeFormatter::AttemptToExtractCountryCode() {
string new_region_code;
phone_util_.GetRegionCodeForCountryCode(country_code, &new_region_code);
if (PhoneNumberUtil::kRegionCodeForNonGeoEntity == new_region_code) {
current_metadata_ =
// Non-geographical metadata can be missing for unrecognised calling codes
// (e.g. the user typing "+999"); fall back to empty_metadata_ so subsequent
// reads such as national_prefix() don't dereference a null pointer while
// we wait for further digits.
const PhoneMetadata* non_geo_metadata =
phone_util_.GetMetadataForNonGeographicalRegion(country_code);
current_metadata_ =
non_geo_metadata != NULL ? non_geo_metadata : &empty_metadata_;
} else if (new_region_code != default_country_) {
current_metadata_ = GetMetadataForRegion(new_region_code);
}
Expand Down
31 changes: 31 additions & 0 deletions cpp/src/phonenumbers/asyoutypeformatter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,37 @@ class AsYouTypeFormatter {

bool CreateFormattingTemplate(const NumberFormat& format);

// Writes an "effective" format string into |number_format| with the
// national-prefix formatting rule applied, so any punctuation supplied by
// the rule (e.g. parens around the area code) survives into the formatting
// template. Mirrors the rule application in
// PhoneNumberUtil::FormatNsnUsingPattern, but limited to the NATIONAL-format
// path: when the formatter is in international mode (is_complete_number_
// with no extracted national prefix), the bare format is written, matching
// what Format(num, INTERNATIONAL) would emit.
//
// Three branches:
// 1. Rule is empty (or we are in international mode): write the bare
// format unchanged.
// 2. The rule begins with the national prefix (e.g. "$NP $FG" after
// metadata-loader substitution -> "8 $1" for RU mobile): strip the
// leading prefix and any adjacent ' ' or '-' separator, then splice
// the remainder into the format. The trunk prefix continues to render
// separately via prefix_before_national_number_, so this avoids
// doubling it.
// 3. The rule contains the prefix in a position we did not strip (e.g.
// GB fixed-line's "($NP$FG)" -> "(0$1)", which wraps the prefix inside
// literal punctuation): write the bare format. Splicing such a rule
// would duplicate the trunk prefix that AYTF renders separately, and
// AYTF has no clean place to suppress that.
//
// By the time this runs, BuildMetadataFromXml has already substituted "$NP"
// with the literal national prefix and "$FG" with "$1", so the rule string
// is a regex-replacement string. Contents are trusted (built by metadata
// tooling) and are spliced via RegExp::Replace without re-quoting.
void ApplyNationalPrefixFormattingRule(const NumberFormat& format,
string* number_format) const;

// Gets a formatting template which could be used to efficiently format a
// partial number where digits are added one by one.
void GetFormattingTemplate(const string& number_pattern,
Expand Down
Loading