It is not the task of a core API function to replace anything in the URI it receives from the application. Please fix the URI in your application before it makes call_connect() call and close this pull request.

You are right.
I will change it in the modules menu and gtk and make it configurable.

I will keep this pull request open for this matter.

Would the changes made to the modules gtk and menu a viable way to address this behavior?

I'm still a little unhappy about the code duplicates and unsure about the line-breaks to keep the lines under 79 characters.

What does your pull request do? Is it just removing spaces from what menu user enters after d or something else?

I added the configuration setting "whitespace_handing". This setting has three allowed values: none, remove, uri_escape. This setting will change spaces enter in the menu entered after d and in the gtk dial menu.

Default behavior is none, which changes nothing.
uri_escape will change spaces into "%20" which is, from my understanding, the RFC compliant way to change the number, but somehow wont work with our phone system.
Remove will just remove all spaces.

Lets say i want to call the white house. After a quick search i find the number is "+1 202-456-1111". If i copy said number and enter it in the menu or gtk and call unaltered nothing happens.
If uri_escape is active, the number will change to "+1%20202-456-1111".
If removed is active, the number will change to "+1202-456-1111" which should work on all phone system.

Module parameter as in build time instead of runtime?

Could you please further elaborate?

It already is in the Module parameters section.

I could probably add two parameters instead of just one (menu_whitespace_handling and gtk_whitespace_handling) to differentiate between the modules.

But since you have also uri_escape alternative, which I assumes escapes
all chars that are not allowed in uri, whitespace does not sound like a
good term.

I only escape whitespace character to date. A full blown uri-escape would be way more complex or would add an additional external library. Both of which seems unnecessary as far as i can see it.

Also, I don't quite understand the need for this. Why would the user
write +1 202-456-1111 instead of +12024561111?

Because people want to copy and paste numbers. Most phone numbers are written with spaces to make them more user readable.

Also, if - chars are left in the userpart, it is unlikely that a sip proxy or
pstn gw would correctly route the request.

At least for my setup removing the white-spaces is sufficient to make the call.

Yes, but I bet that in many other SIP Proxy/PSTN Gateway setups - chars
would not be accepted. Can you point to some ITU or other standard
that tells that - chars are OK in E.164 numbers?

You are right with this. But since in the menu and the gtk modules it is
allowed to call based on sip aliases (john-doe) removing those characters
cannot be simply done.
So I rewrote the pull request again. :)

• two new module parameters in the config
  • gtk_clean_number | bool | default no
  • menu_clean_number | bool | default no
• if the dialed number has an alphabetical character (A-Za-z) the dialed "number" will not be altered
• else
  • it will remove (0) after the country code (if there is one)
  • remove all ' ', '-', '/', '(' and ')' characters from the number

Most phone numbers are written with spaces to make them more user readable.

It's not only about readability here, but also about international standards compliance, because:

• German DIN 5008 requests the written form "030 12345-67" for functional structuring, but also "0900 5 123456" to properly display the classification number (here: 5) of premium rate services
• E.123 requests "(030) 12345 67" for example as written form

However, "+49 (0) 30 12345-67" is simply a wrong German and Austrian writing, unfortunately very common but still violating all standards, even German DIN 5008. In order to support copy & paste from those zillions of websites (or e-mail footers), I highly appreciate your cleanup option (because after your cleanup this leads to a proper "+49301234567" from what I understand).

As per RFC 5733 (based on E.164), there is however also "+49.301234567", which doesn't seem to be covered so far. It would be outstanding, if you could extend your PR here slightly.

• else
  • it will remove (0) after the country code (if there is one)

Shouldn't that be int i = 2, k = 0;? Because the shortest international code has one digit, thus "+7(0)" should be the shortest possible constellation.

Philosophical: I'm not sure whether str[i] && i < 7 is perfect, but for the common abuse situation in Germany and Austria, it's absolutely suitable. Note: "+61 8 9162" is the international code of Cocos Islands (but I'm heavily in doubt whether they ever will use the broken German format "+61 8 9162 (0) 1234567").

As per RFC 5733 (based on E.164), there is however also "+49.301234567", which doesn't seem to be covered so far. It would be outstanding, if you could extend your PR here slightly.

I extended the PR to also remove dots from the clean number

Shouldn't that be int i = 2, k = 0;?

Since i loop over the pointer I want to start at the beginning of the string, since the user could have simply entered '+' which then could lead to unexpected behavior. I could check the strlen of the given number but then i would have to include string.h in both changed modules.

Philosophical: I'm not sure whether str[i] && i < 7 is perfect, but for the common abuse situation in Germany and Austria, it's absolutely suitable. Note: "+61 8 9162" is the international code of Cocos Islands (but I'm heavily in doubt whether they ever will use the broken German format "+61 8 9162 (0) 1234567").

I altered the PR that the (0) now has to be followed by another character in order to remove it and i only remove the first occurrence of (0). So +61 8 9162 (0) 1234567 will now be determined and cleaned up correctly while +61 8 9162 987654321 (0) in which (0) is used as an direct dialing hint the 0 will be keept in the cleaned up number.

In my opinion this is getting far too complicated. There exists tons of different dialing plans in different countries. Better to stick to the original idea and just remove white space from whatever user enters after d or in the corresponding gtk thing in order to make it compliant with sip uri userpart syntax.

I agree that there are various dialplans worldwide, however the "+49 (0) 30 12345-67" is unfortunately a very common wrong way in Germany and Austria to write a phone number; it even has nothing to do with the dialplan. People would like to express with "+49 (0)" that you shall dial either "+49" or "0". So I still would really like a simple way to strip the wrong "(0)" if it appears after the country code.

Firstly I found a bug in my gtk module changes that I have to fix asap. gtk saves the cursor
position, which can be at a spot which no longer exists due to the shortening of the number.

juha-h writes:

In my opinion this is getting far too complicated. There exists tons of different dialing plans in different countries. Better to stick to the original idea and just remove white space from whatever user enters after d or in the corresponding gtk thing in order to make it compliant with sip uri userpart syntax.

But you made good and valid points, that just removing whitespaces will not make
inputs E.164 compliant numbers. If you think removing numbers which are potentially
wrong inputs should not be done, I can remove this feature from the PR, even through
it would help me a lot. I am just so used to this writing of phone numbers, that I did not
consider that it might be a regional problem.

juha-h writes:

Menu and gtk modules are application modules. You could write German
version of them for your users.

I would rather not, since then future updates to the Menu modules also have to migrated
into the german version.

juha-h writes:

Another option could be to introduce a a configuration variable that has regex value and that regex is then applied to the string that user enters after d.

While i kind of like the idea of giving the user more freedom, this would be way more
complicated to implement and has way more ways to fail than this solution.

juha-h writes:

re lib has regex functions that could be used. i don't get why there
would be more ways to fail.

I have used re_regex in the PR.
From the documentation of re_regex:

 *   We parse the buffer for any numerical values, to get a match we must have
 *   1 or more occurences of the digits 0-9. The result is stored in 'num',
 *   which is of pointer-length type and will point to the first location in
 *   the buffer that contains "42".

Lets say our phone number is +1 202-456-1111 and the user regex is [+0-9]+.
After the re_regex call it would return +1. Now i have to remove the matched
part from the number somehow and call re_regex again on the remaining string
while building a new string from the results.
Other problems are:

• removing (0) is not really possible with this solution
• re_regex does not offer a full PCRE which can be confusing for the user

Another possibility would be to give the user regex expression of strings he wants
to remove from the phone number. E.g. it would be a list {"(0)", " ", "-"}. Now i could
find with re_regex that the user given strings are in the phone numbers, but still
would have to exclude them from the phone number somehow, and still have to
make multiple re_regex calls until no more results for each given string are found.

It would also be possible to take the list and use the negation before calling re_regex.
Again i would have to loop over the given number multiple times build and and walk
though multiple char arrays.

This added complexity and the component, that the user has no idea, which regex he
is allowed to enter means that there are way more ways to fail.

I fixed the gtk bug.

I wanted to remove the logic which determines the "(0)" too, but after some research
this formation is more common then Robert-scheck describes it and I want to
discuss it again.

Some examples:
Germany:
University Rostock: https://www.msf.uni-rostock.de/fakultaet/ansprechpartner/fakultaetsleitung/
Airline Condor: https://www.condor.com/de/popup_hotline.jsp
Sweden:
EMTUNGA: https://www.emtunga.com/contact-us-1
Stockholm University: https://www.su.se/cmlink/stockholm-university/contacts
Spain:
Huf Group: https://www.huf-group.com/companies/huf-espana
UPS: https://www.ups.com/es/en/help-center/contact.page
UAE:
https://www.etihad.com/en-us/legal/terms-and-conditions-us/customer-service-plan
USA:
https://www.pickit3d.com/certified-channel-partners
Finland:
https://www.huhtamaki.com/en/about/contact-us/

I my opinion whatever is done, it needs to be a generic solution, not
something that handles a few national formats.

I wanted to illustrate with my examples that it is a common mistake and not just a few national formats.

So as far as i can see there are three possibilities.

1. I could make the introduced gtk_clean_number and menu_clean_number not booleans but enums instead. It would allow no, yes and experimental.
   • no: same behavior as right now
   • yes: remove .() /- from numbers
   • experimental: remove (0) from the number also
2. I remove the possibility to remove the (0) altogether. Meaning gtk_clean_number and menu_clean_number can be
   • no: same behavior as right now
   • yes: remove .() /- from numbers
3. I close the PR and hope for someone else to address this Issue, because using libpcre clean up the number and having to describe the options in the limited space within the config file is not something i feel capable of doing.

can someone else please test this patch, and check that dialing name/numbers
is still working ?

@mbattista your patch introduces a lot of unnecessary whitespace/line
additions/removals. Please check your own patch again, and verify that it is clean.

I cleaned up all unnecessary whitespaces.

With this line I hope that no changes will be done when names will be dialed.
https://github.com/baresip/baresip/pull/1007/files#diff-1bf96c019a0bc23fcceec7262bc6122bR26
When there are only numbers it should not remove any important information from the number.

Nonetheless I hope that some else can test this patch.

it would be nice if someone could test this patch.

one week has passed, and no one has stepped up to test it.

is there really such a big need for this patch, as you claim?
if there was a big need for it, people would take 10 minutes
of their time to test the PR.

is there really such a big need for this patch, as you claim?

Well, for me it was annoying, that I could not copy and paste phone numbers into
gtk or menu and just call them. I always had to check for spaces first or the call
would not connect. So I fixed it, by removing spaces and later also .()/- from the
dialed number and submitted this PR.

Since it changes user input, I tried to make the changes as non-intrusive as I
possibly could. Meaning, that the change is an optional setting, which is disabled
by default and that I would not replace anything as soon as there is an alphabetical
character in the dialed number.

This means a baresip user can enable it to use the feature and if he finds a
bug or an unexpected behavior he could disable it again, make an Issue and use
it like he was before he had enabled it.

Like i said before, if you feel that this PR would not improve anything or does not
meet the standards, I can close it and open an issue instead so that someone else
can address this.

it would be nice if someone could test this patch.

I will test this patch for sure, but most likely next week; I did not have any spare time last week nor will I have spare any time this week for my hobby projects, sorry.

@mbattista, feel free to remind me next week, if I did not provide any test results until next week Tuesday evening.