nkh / P5-App-Asciio

Plain ASCII diagram
https://nkh.github.io/P5-App-Asciio/
53 stars 4 forks source link

about unicode_length and markup mode #110

Closed qindapao closed 10 months ago

qindapao commented 1 year ago

@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 commented 1 year ago

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:

https://github.com/nkh/P5-App-Asciio/blob/3df8674a5d6ff29acb7f8e663a2d58c03a8e0639/lib/App/Asciio/String.pm#L72-L82

qindapao commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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:

nkh commented 1 year ago

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 commented 10 months ago

df584268a85b97a54e9bd336e77a9a5a45ddd318

@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 commented 10 months ago

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