about unicode_length and markup mode

[qindapao]

@nkh

I noticed that you added the function get_unicode_length, which will handle the regularization of markup mode.

It is also needed for element width calculation, but I don't see it used in many places. Using unicode_length without deleting the mark will cause the width of the element in mark mode to become wider (to calculate the width of the element needs to delete the mark) .I suggest that I fix it first and then think about the markup mode thing.

Almost all places where unicode_length is used need to be replaced with get_unicode_length, except for a few places

[nkh]

I think the Markdown mode needs a refactoring, it's a bit all over the place. We succeeded making the crossing mode part of Asciio and we can do the same for different types of Markdown.

Please read #102, #92 and #54 so we can have a generic way of handling all types of extra information in elements. We'll also need to think about they are edited and visualized, you've made editing inline and visualization inline, that works just for your needs and we can't easily add other types. I think we need to decouple the Markdown from Asciio.

Hopefully you'll still have inline editing and rendering but it needs to be done in a "plugin" not in the middle of Asciio's code.

Maybe the simplest it to move the markdown code to a Zwiki_Markdown.pm module and replace it with stubs calling the MARKDOWN plugin; that way we can define in the configuration what markdown is used, preferably per element.

You can't just use get_unicode_length everywhere you have MARKDOWN because you need an Asciio object, beware that the $self refers to elements in a lot of places not an Asciio object; the stripes know nothing about Asciio.

This needs a solid design, As a proof of concept your code is good but we need a more generic solution for a good integration we can base on your code.

The differences between unicode_length and get_unicode_length are:

• unicode_length is a pure unicode_length function, it doesn't care about MARKDOWN or Asciio and it should be kept that way
• get_unicode_length needs an object and filters out MARKDOWN
  • the MARKDOWN should be handled somewhere else but it was the cleanest I could while we were refactoring crossing mode
  • the MARKDOWN should be handled in the MARKDOWN "plugin"

P5-App-Asciio/lib/App/Asciio/String.pm

Lines 72 to 82 in 3df8674

	package App::Asciio ;
	sub get_unicode_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::unicode_length($string) ;
	}

[qindapao]

@nkh

Ok, I see what you mean. I need to spend some time thinking about it. If you already have a plan, you can also start to act. At present, the markup mode is a semi-finished product. It seems that we either implement it in 1.9, or remove it completely, including the documentation, and wait for 2.0 to complete the development?

[nkh]

@qindapao I have idea ... but no plan yet.

I think it's a bit larger work, IE not one day, if you don't need it in 1.9 we can move it to another release, otherwise we keep it in.

In any case I wrote that there was a problem with bold font rendering, the edge of the box is not aligned, you can fix that if you want, it will be used later anyway.

[qindapao]

@nkh

Then keep the code first, and inform the user that there are still some problems and it is not recommended to use it. The bold problem may involve the core of GTK3 font library rendering, and I haven't found where the problem is.

[nkh]

If we keep it in 1.9 it will still need to be refactored.

There are two phases to the refactoring, which can be done in different order:

• refactor the current code so it's kept in it's own module
  • it's ok to calls in, for examples, Asciio::GTK, but not inlining all the code there
• support multiple markdown not just zwiki
  • may need a new type of element, makes little sense to check arrows text for markdown for example

[nkh]

And there should not be a MARKUP_MODE at all, either the elements have markup that needs to be handled or they don't.

Asciio know little about elements, they have a size and when they need to be rendered they return stripes, so neither MARKUP_MODE, nor code handling the markup should be in Asciio, it should be in the elements.

[qindapao]

df58426

@nkh It's been a long time since the last code review. I recently took a vacation, which gave me some time and my health is getting better.The above are some fixes I made to markup mode based on your code review comments.Although they are not perfect, they are still in-line modes, but you can see if they meet the requirements in 1.9. I plan to completely refactor it in 2.0.

[nkh]

@qindapao I've merged (21 files changes! nothing should have that impact) ; let's move the rest of the changes to 2.0.