Refactor Toolfun.pm

[nkh]

Name says little.

Toolfunc has these functions:

44  sub set_double_width_qr_and_markup_mode
 52  sub usc_length
 65  sub make_vertical_text
103  sub new_box
121  sub new_wirl_arrow
139  sub add_connection
182  sub move_named_connector
229  sub optimize_connections
237  sub get_canonizer
248  sub register_hooks

it's a little bit of a mix, and move_named_connector is not used at all.

[nkh]

@qindapao I notice now that most of the function here are from the scripting library.

In an effort to make loading of Asciio faster, I didn't include those functions in the main modules but I don't think it makes a big difference.

I propose I start the refactoring of ToolFun.pm, now that I know it's should be called Scripting.pm, and document it.

That would leave just a few function for you to look at, would that work for you?

[qindapao]

@nkh

The biggest trouble is usc_length.

I'm not sure about the details of your plan yet. Maybe I can understand your plan better if you write a sample code.

[qindapao]

@nkh

You can write according to your idea first, and then we'll see what's wrong together.

[nkh]

5061180

@qindapao I've refactored the whole scripting modules and added a new simplify interface, the documentation is here https://github.com/nkh/P5-App-Asciio/blob/master/documentation/mdbook_asciio/src/for_developers/scripting.md

the TooFunc module is almost empty now https://github.com/nkh/P5-App-Asciio/blob/master/lib/App/Asciio/Toolfunc.pm and I have changed the way to set the double_width_qr.

I leave the next phase to you:

-rename the module I propose one of these, chose or propose something better

• Asciio::Static (because they are all static functions)
• Asciio::String (because they all deal with strings)
• Asciio::StaticText
  • change the references everywhere it's now included and please check if it's really necessary to include them at all

[qindapao]

https://github.com/qindapao/P5-App-Asciio/commit/d363aee33fac158956c3d4c8c6c0de2b12f251c8

@nkh

Thai, Arabic and Hebrew are now supported, and the biggest change is usc_length rewriting. Now there is no need for users to configure complex regular strings!

DOUBLE_WIDTH_QR => qr/
            [\x{3400}-\x{4db5}] |
            [\x{4e00}-\x{9fa5}] |
            [\x{ac00}-\x{d7ff}] |
            [\x{3008}-\x{3011}] |
            [\x{2014}\x{2026}\x{3001}\x{3002}\x{3014}\x{3015}\x{FF01}\x{FF08}\x{FF09}\x{FF0C}\x{FF0E}\x{FF1A}\x{FF1B}\x{FF1F}\x{FF0F}\x{FF3C}]
            /x,

The above thing has been removed by me.

I've also updated the full documentation on unicode support, both for users and developers

[qindapao]

You can try to see if it can work and align normally in your place, pay attention to use the default monospace font, not sarasa mono sc

check.zip

The default monospaced font on my side can align Thai, Arabic and Hebrew

[nkh]

I merged it, via github pull request ... I shouldn't have lol! I always say that's it a terrible idea to not merge locally and test first but the patched looked good. Unfortunately it doesn't work as expected on my machine.

It's probably a font problem. What's the name of you default font? Better yet include a copy as well as a copy of the Asciio file where you have the multiple languagues, check.zip you included above only contained Thai and Latin.

[qindapao]

@nkh

I am in the company today, and I will go back to look at my font in the evening.

In addition, I found a little defect of markup mode, as you demonstrated.There is an underlined box that is not aligned.It is related to the variables stored in the object and the variables in the configuration.I'll analyze why usc_length didn't get this variable from the object.

[qindapao]

USE_MARKUP_MODE

When this variable is passed into the use_markup function, it does not use the variable in the object but the variable in the configuration.

I don't know why.

As for the three new language examples and fonts, it should be a small problem. I'll go back and make sure at night.

[qindapao]

@nkh

How about I save asciio's reference directly in String.pm?

So I don't have to worry about the use_markup variable getting the wrong value.

It should have been obtained from the saved object, but it did get the value in the configuration.

[nkh]

We keep your commit as the new master in my repo, it's merged and sometimes backing to previous version is worse than keeping the one that has a small problem. So your latest commit is the master in my repo.

As you've noticed I removed the reference to the Asciio object in my refactoring, because there's no reason for it to be there. If the code code in String.pm need to know about USE_MARKUP_MODE then it shouldn't know about the whole universe.

You wrote "When this variable is passed into the use_markup function, it does not use the variable in the object but the variable in the configuration" ... the configuration data is the object

https://github.com/nkh/P5-App-Asciio/blob/8dac6dc33486ad7977070c21cbf1e71f0c35828a/lib/App/Asciio/Setup.pm#L431-L458

[qindapao]

@nkh Well, I understand your principles.

Here should be to get the priority of variables, first take variables from the configuration, and then pass them to the function. The transfer function was not called for the second time when the variable was taken from the saved file later.

It's not a big problem either. I'll write it down first. There should be a better plan when mark up is refactor later.

[qindapao]

The variables in the configuration are different from those saved in the asciio file.

[nkh]

Yes, Markup needs a refactoring #102 which I put in milestone 2.0 but maybe it should be in 1.9.

The only way I can think about having access directly to the object is to make usc_length part of the object, that's fine for me, and change the 50 lines of code where it's used.

Even in that case I think the line below should be changed as part of the refactoring when markup is made generic

$string =~ s/<span link="[^<]+">|<\/span>|<\/?[bius]>//g if $use_markup ;

If you mean that USE_MARKUP_MODE is not present in the saved "diagram.asciio" file then it's a bug! I'll have a quick look now.

[qindapao]

@nkh

It is in it, but it is passed to the function with the values in the configuration, and the values in the asciio file have not been overwritten.

[qindapao]

@nkh

Maybe the function that sets the value was called too early.

[qindapao]

@nkh

I suggest it's still in 2.0. If it affects the release of 1.9, we can delete this part of the code first.

I've been getting busy recently.And it involves the renewal of the company contract. I have to balance work and hobbies.

sorry

[nkh]

It's not called at all!

The problem is not with how we setup the object, as I wrote above the config and the object are merged together and it becomes the object.

The problem is how we load the object when a file is read. Inside the file there's a serialized object, it's loaded and used but the code to set markup mode is not called.

App::Asciio::Actions::File::Open need to have a call to use_markup() in it, as is done when modifying an Asciio object.

https://github.com/nkh/P5-App-Asciio/blob/8dac6dc33486ad7977070c21cbf1e71f0c35828a/lib/App/Asciio.pm#L659C4-L671

Even when you had access to the Asciio object, Markup was not part of Asciio, we just used the object to keep the config.

This problem will be fixed automatically when Markup is refactored so usc_length uses the object value instead for a the $use_markup config we use.

In the meantime, calling use_markup() in App::Asciio::Actions::File::Open will fix the problem.

We don't need to do all the Markup refactoring in one shot (ie: the generalization can be next phase) but we can make usc_length part of Asciio first.

What we will need is two functions, the one we already have but is external to Asciio, and one in Asciio that calls it. Give me 30 min and I'll post some code.

In any case, I don't see how USE_MARKUP_MODE is involved in Unicode display problem.

[qindapao]

In any case, I don't see how USE_MARKUP_MODE is involved in Unicode display problem.

@nkh USE_MARKUP_MODE Will affect alignment.If the variable is not set, the mark character will not be deleted, and the calculation of offset will be too much.

[nkh]

I see that in the code but usc_length should not be called with markup in it, we're using one sub to do two things.

To fix it in the 1.9 release we need to:

• modify the String::usc_length
• add a usc_length (what does "usc" stand for?) in the Asciio base object
• change all the calls to usc_length(...) to $self->usc_length(..)

101 may be interesting to know about even if I put it in release 2.1

# in String.pm (so we can reuse it somewhere else)

sub usc_length
{
my ($string) = @_ ;

# reference to MARKUP removed!

my $east_asian_double_width_chars_cnt = grep {$_ =~ /\p{EA=W}|\p{EA=F}/} split('', $string) ;
my $nonspacing_chars_cnt = grep {$_ =~ /\p{gc:Mn}/} split('', $string) ;

return length($string) + $east_asian_double_width_chars_cnt - $nonspacing_chars_cnt ;
}

# __also__ in String.pm but in Asciio namespace

package App::Asciio ;

sub usc_length
{
my ($self, $string) = @_ ;

# Markup is not part of Unicode, handle it in Asciio
$string =~ s/<span link="[^<]+">|<\/span>|<\/?[bius]>//g if $self->{USE_MARKUP_MODE} ;

return App::Asciio::String::usc_length($string) ;
}

[nkh]

@qindapao I can make the change above on top of your branch if you want to, I use vim and can do that quickly.

[qindapao]

@nkh

Ok, I'll try. Besides, usc is the name you gave, and you forgot it yourself.

I used to take phy_length.It's not accurate either,I think it should be called print width.

[qindapao]

@nkh

You do it. I'm still in the company. I communicate with you on my mobile phone.

[nkh]

I remember now, it is the base system of unicode. and I gave you the wrong name it should have been ucs not usc!

in Unicode parlance

• length: the number of characters
• byte_length: the number of bytes

we can't use Perl's length as we may have mixed Unicode in the code itself and inside the elements, pretty cool.

I'll change the function name to unicode_length.