Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't cut off text for addendum in mid-word #607

Merged
merged 10 commits into from
Nov 10, 2022
66 changes: 35 additions & 31 deletions docassemble/AssemblyLine/al_document.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
alpha,
)
from docassemble.base.pdfa import pdf_to_pdfa
from textwrap import wrap

__all__ = [
"ALAddendumField",
Expand Down Expand Up @@ -141,6 +142,7 @@ def overflow_value(
preserve_newlines: bool = False,
input_width: int = 80,
overflow_message: str = "",
preserve_words: bool = True,
):
"""
Try to return just the portion of the variable (list-like object or string)
Expand All @@ -157,6 +159,7 @@ def overflow_value(
input_width=input_width,
preserve_newlines=preserve_newlines,
_original_value=original_value,
preserve_words=preserve_words,
)
if isinstance(safe_text, str):
# Always get rid of double newlines, for consistency with safe_value.
Expand Down Expand Up @@ -201,7 +204,8 @@ def safe_value(
overflow_message: str = "",
input_width: int = 80,
preserve_newlines: bool = False,
_original_value=None,
_original_value: Optional[str] = None,
preserve_words: bool = True,
):
"""
Try to return just the portion of the variable
Expand Down Expand Up @@ -234,43 +238,43 @@ def safe_value(
)
max_chars = max(self.overflow_trigger - len(overflow_message), 0)

# If there are at least 2 lines, we can ignore overflow trigger.
# each line will be at least input_width wide
# If we preserve newlines, we need to account for max_lines, not just max_chars
if preserve_newlines and max_lines > 1:
if isinstance(value, str):
max_lines = self.max_lines(
input_width=input_width, overflow_message_length=0
nonprofittechy marked this conversation as resolved.
Show resolved Hide resolved
)
max_chars = self.overflow_trigger
# Replace all new line characters with just \n. \r\n inserts two lines in a PDF
value = re.sub(r"[\r\n]+|\r+|\n+", r"\n", value).rstrip()
line = 1
retval = ""
paras = value.split("\n")
para = 0
while line <= max_lines and para < len(paras):
# add the whole paragraph if less than width of input
if len(paras[para]) <= input_width:
retval += paras[para] + "\n"
line += 1
para += 1
else:
# Keep taking the first input_width characters until we hit max_lines
# or we finish the paragraph
while line <= max_lines and len(paras[para]):
retval += paras[para][:input_width]
paras[para] = paras[para][input_width:]
line += 1
if not len(paras[para]):
para += 1
retval += "\n"
# TODO: check logic here to only add overflow message when we exceed length
if len(paras) > para:
return (
retval.rstrip() + overflow_message
) # remove trailing newline before adding overflow message
else:
return retval
# textwrap.wrap does all the hard work for us here
return " ".join(
wrap(
value,
width=input_width,
max_lines=max_lines,
replace_whitespace=False,
placeholder=overflow_message,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how this would interact with newlines in value, which people are likely to add in text areas I think.https://docs.python.org/3/library/textwrap.html#textwrap.TextWrapper.replace_whitespace says that it'll "cause strange output", which makes me a little hesitant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't cause strange output in our usage, which ends up removing the line breaks after any get added.

We just use textwrap.wrap to figure out how many lines should fit, but we let the pdf renderer handle the actual soft line breaks in the output.

Copy link
Contributor

@BryceStevenWilley BryceStevenWilley Nov 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't removing all of the line breaks, we're just collapsing things where multiple appear in a row. Test overflow of 90, and input:

At least 10% of people 

in this target audience do not know what this term means.
 
Therefore, you must avoid this term, explain this term, or link to an explanation.

Results in 3 lines of text, when there should only be 2, because there are still some \ns inside the input text. Extra lines like that could cause things to get cut off in the PDF by pushing the last line below the height of the text box, right?

)
).replace(" ", " ")

# Strip newlines from strings
# Strip newlines from strings because they take extra space
if isinstance(value, str):
if len(value) > self.overflow_trigger:
if preserve_words:
return (
next(
iter(
wrap(
value,
width=max_chars,
replace_whitespace=True,
drop_whitespace=True,
)
)
)
BryceStevenWilley marked this conversation as resolved.
Show resolved Hide resolved
+ overflow_message
)
return (
re.sub(r"[\r\n]+|\r+|\n+", " ", value).rstrip()[:max_chars]
+ overflow_message
Expand Down
28 changes: 26 additions & 2 deletions docassemble/AssemblyLine/test_al_document.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import unittest
from docassemble.base.util import DAFile
from .al_document import ALDocument, ALDocumentBundle
from docassemble.base.util import DAFile, define
from .al_document import ALDocument, ALDocumentBundle, ALAddendumField


class test_dont_assume_pdf(unittest.TestCase):
Expand Down Expand Up @@ -33,5 +33,29 @@ def test_upload_pdf(self):
pass


class test_aladdendum(unittest.TestCase):
def test_safe_value(self):
TEXT = """
Charged by my father with a very delicate mission, I repaired, towards the end of May, 1788, to the château of Ionis, situated a dozen leagues distant, in the lands lying between Angers and Saumur. I was twenty-two, and already practising the profession of lawyer, for which I experienced but slight inclination, although neither the study of business nor of argument had presented serious difficulties to me. Taking my youth into consideration, I was not esteemed without talent, and the standing of my father, a lawyer renowned in the locality, assured me a brilliant patronage in the future, in return for any paltry efforts I might make to be worthy of replacing him. But I would have preferred literature, a more dreamy life, a more independent and more individual use of my faculties, a responsibility less submissive to the passions and interests of others. As my family was well off, and I an only son, greatly spoiled and petted, I might have chosen my own career, but I would have thus afflicted my father, who took pride in his ability to direct me 4in the road which he had cleared in advance, and I loved him too tenderly to permit my instinct to outweigh his wishes.

It was a delightful evening in which I was finishing my ride on horseback through the woods that surrounded the ancient and magnificent castle of Ionis. I was well mounted, dressed en cavalier, with a species of elegance, and accompanied by a servant of whom I had not the slightest need, but whom my mother had conceived the innocent idea of giving me for the occasion, desiring that her son should present a proper appearance at the house of one of the most brilliant personages of our patronage.

The night was illuminated by the soft fire of its largest stars. A slight mist veiled the scintillations of those myriads of satellites that gleam like brilliant eyes on clear, cold evenings. This was a true summer sky, pure enough to be luminous and transparent, still sufficiently softened not to overwhelm one by its immeasurable wealth. It was, if I may so speak, one of those soft firmaments that permit one to think of earth, to admire the vaporous lines of narrow horizons, to breathe without disdain its atmosphere of flowers and herbage—in fine, to consider oneself as something in this immensity, and to forget that one is but an atom in the infinite.

In proportion as I approached the seigneurial park the wild perfumes of the forest were mingled with those of the lilacs and acacias, whose blooming heads leaned over the wall. Soon through the shrubbery I saw the windows of the manor gleaming behind their curtains of purple moire, divided by the dark crossbars of the frame work. It was a magnificent castle 5of the renaissance, a chef-d’œuvre of taste mingled with caprice, one of those dwellings where one is impressed by something indescribably ingenious and bold, which from the imagination of the architect seems to pass into one’s own, and take possession of it, raising it above the usages and preoccupations of a positive world.

I confess that my heart beat fast in giving my name to the lackey commissioned to announce me. I had never seen Madame d’Ionis; she passed for one of the prettiest women in the country, was twenty-two, and had a husband who was neither handsome nor amiable, and who neglected her in order to travel. Her writing was charming, and she found means to show not only a great deal of sense, but still more cleverness in her business letters. Altogether she was a very fine character. This was all that I knew of her, and it was sufficient for me to dread appearing awkward or provincial. I grew pale on entering the salon. My first impression then was one of relief and pleasure, when I found myself in the presence of two stout and very ugly old women, one of whom, Madame the Dowager d’Ionis informed me that her daughter-in-law was at the house of her friends in the neighborhood, and probably would not return before the next day.

“You are welcome, all the same,” added this matron. “We have a very friendly and grateful feeling for your father, and it appears that we stand in great need of his counsel, which you are without doubt charged to communicate to us.”

“I came from him,” I replied, “to talk over the affair with Madame d’Ionis.”
"""
define("fake_value", TEXT)
myfield = ALAddendumField(field_name="fake_value", overflow_trigger=120)
self.assertLessEqual(
len(myfield.safe_value(overflow_message=" [See addendum]")), 120
)
BryceStevenWilley marked this conversation as resolved.
Show resolved Hide resolved


if __name__ == "__main__":
unittest.main()