Simplify specifying the new position after multline text

[gmischler]

When having a closer look at how to best adapt .write() to the new line wrapping code (per #340), I ended up struggling a bit with the concept behind the ln=# parameter to both .multicell() and .write(), and now also ._render_styled_cell_text(). Judging by its name, I suspect that originally it was only meant as a True/False value to indicate wether the position should move to a new line. Later it seems to have been increasingly overloaded to also indicate horizontal movement (or maybe it was badly designed right from the start, who knows...) Either way, using an arbitrary single numerical value to indicate movement choices in both x and y is rather confusing and unintuitive.

To improve the clarity of the API and also simplify maintenance, I think it would make sense to seperate the two functionalities. The simplest way is probably to introduce a seperate argument new_xpos="", with a single character value choice of "L|R|S|E", for Left/Right of the requested cell extents, or Start/End of the actual text rendered.

The ln= parameter could then again be reduced to a True|False choice of moving to a new line or not, ideally with a deprecation warning for other uses.

Any thoughts? Better names for the new parameter?

[gmischler]

On closer inspection, it's even worse than I first thought, since .multi_cell() actually uses different semantics than the other two (assuming we can trust the docstrings):

_render_styled_cell_text()
	`0`: to the right;
	`1`: to the beginning of the next line
	`2`: below. 

cell()
	`0`: to the right;
	`1`: to the beginning of the next line;
	`2`: below. 

multi_cell()
	`0`: to the bottom right;
	`1`: to the beginning of the next line;
	`2`: below with the same horizontal offset;
	`3`: to the right with the same vertical offset.

And no, a non-true value doesn't necessarily indicate no line jump. This conflicts with reusing ln=False for staying on the same line.
So we would need a different parameter for the vertical displacement as well. Would it be too confusing to have both ln=# and new_line=True available at the same time? Maybe add_line/next_line/trailing_nl/whatever?

[Lucas-C]

Thank you for opening this reflection @gmischler

One important thing to keep in mind is backward compatibility.
If ln gets transformed into a boolean, that means that values of 1 / 2 / 3 will be considered to simply be True in the next fpdf2 release. This change would cause very unpleasant surprises for users, and should either be avoided, or we would need to check for integer values passed to all those functions in order to raise a warning and be backward-compatible.

Also, the default behaviour (without ln specified) of those functions should not change.

I think you are right, API clarity could be improved here, and I'm open to deprecating the ln parameter.
My suggestion would be a replacement eol ("end of line") parameter that will take an IntEnum value with very explicit names for its constants.

[gmischler]

One important thing to keep in mind is backward compatibility.
...
Also, the default behaviour (without ln specified) of those functions should not change.

No doubt about either of those.
I just hadn't realized at first, how little sense the specific values for ln=# really make. By now it's clear that reusing it won't work, though.

My suggestion would be a replacement eol ("end of line") parameter that will take an IntEnum value with very explicit names for its constants.

"End of..." is a helpful concept! But maybe eot as in "end of text" might be closer to the actual semantics.

Do you think we can get away with just one new parameter? After all, we have two directions to deal with:

Horizontally (eot_x):

• left of cell
• right of cell
• start of text
• end of text
• center of text

Vertically (eot_y):

• top of block
• bottom of block (= top of next line)
• top of last line (for continuing the line)

I'm just brainstorming right now, so some of those may not have much practical value...
But is there a useful way to combine them into a single value without making things unnecessarily convoluted?

Or are you actually suggesting to turn the ln=# values into such an IntEnum? That would open the possibility to add more, even if still subject to the combinatorics question. (didn't check the code if .cell(ln=0) really behaves differently (stays on the same line) than .multi_cell(ln=0), which would disqualify this option.)

[Lucas-C]

eol = "end of text" sounds good IMHO 😊

If we want to support all combinations of the values for eot_x & eot_y you listed, that would be 5 x 3 = 15 options.
If we want to support those 15 possible behaviours, the solution of having two distinct eot_x & eot_y parameters seems right.
But do we really want to support all those cases?
So far, I do not recall any user asking for more than the 3/4 possible options actually available with ln.

[gmischler]

If we want to support those 15 possible behaviours, the solution of having two distinct eot_x & eot_y parameters seems right. But do we really want to support all those cases? So far, I do not recall any user asking for more than the 3/4 possible options actually available with ln.

Build it and they will come! 😉

I'm trying to think about the most easily accessible and understandable API at the moment.
Whether there will be some fewer or some more options in the end... They don't really cost anything, do they?

[Lucas-C]

No they don't cost much :) It's all about ease of use I think.

[gmischler]

Along the same lines...

I see that .cell() and ._render_styled_cell_text() have a center=False parameter, which when set to True overrides all other positioning and alignment settings and centers the text between the page margins.
This is unnecessarily redundant and confusing. It clutters the code, and the user-side replacement is a trivial one-liner.
Can we pretty please get rid of that as well?

[Lucas-C]

Seems OK with me if align="C" does the exact same thing 😊
Go on and submit a PR ! 😉