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

Fancy format crashes on low linewrap values #1201

Closed
micahellison opened this issue Feb 27, 2021 · 5 comments · Fixed by #1219
Closed

Fancy format crashes on low linewrap values #1201

micahellison opened this issue Feb 27, 2021 · 5 comments · Fixed by #1219
Labels
bug Something isn't working

Comments

@micahellison
Copy link
Member

Bug Report

Environment

  • jrnl --diagnostic output:
jrnl: v2.7
Python: 3.8.2 (tags/v3.8.2:7b3ab59, Feb 25 2020, 23:03:10) [MSC v.1916 64 bit (AMD64)]
OS: Windows 10
  • Install method: source

Current Behavior

When the linewrap config value is particularly low, the fancy formatting crashes with a stack trace that leads to the following error:

ValueError: invalid width -1 (must be > 0)

Expected Behavior

jrnl shouldn't crash. It should be able to gracefully work around these restrictions with wrapping. Maybe part of the solution would be distancing the title and dates from each other so that they don't have to fight each other over the same horizontal length.

Repro Steps

I created a default journal with the following config properties:

    timeformat: '%F %r'
    linewrap: 23

Then I ran:

jrnl -2 --format fancy

Other Information

Full stack trace:

Traceback (most recent call last):
  File "C:\cache\pypoetry\virtualenvs\jrnl-g_ywnB0W-py3.8\Scripts\\jrnl", line 5, in <module>
    cli()
  File "C:\dev\jrnl\jrnl\cli.py", line 35, in cli
    return run(args)
  File "C:\dev\jrnl\jrnl\jrnl.py", line 66, in run
    search_mode(**kwargs)
  File "C:\dev\jrnl\jrnl\jrnl.py", line 172, in search_mode
    _display_search_results(**kwargs)
  File "C:\dev\jrnl\jrnl\jrnl.py", line 327, in _display_search_results
    print(exporter.export(journal, args.filename))
  File "C:\dev\jrnl\jrnl\plugins\text_exporter.py", line 79, in export
    return cls.export_journal(journal)
  File "C:\dev\jrnl\jrnl\plugins\fancy_exporter.py", line 76, in export_journal
    return "\n".join(cls.export_entry(entry) for entry in journal)
  File "C:\dev\jrnl\jrnl\plugins\fancy_exporter.py", line 76, in <genexpr>
    return "\n".join(cls.export_entry(entry) for entry in journal)
  File "C:\dev\jrnl\jrnl\plugins\fancy_exporter.py", line 46, in export_entry
    title_lines = w.wrap(entry.title)
  File "c:\users\micah\.pyenv\pyenv-win\versions\3.8.2\lib\textwrap.py", line 354, in wrap
    return self._wrap_chunks(chunks)
  File "c:\users\micah\.pyenv\pyenv-win\versions\3.8.2\lib\textwrap.py", line 248, in _wrap_chunks
    raise ValueError("invalid width %r (must be > 0)" % self.width)
ValueError: invalid width -1 (must be > 0)

@micahellison micahellison added bug Something isn't working 🆕 New! labels Feb 27, 2021
@sriniv27
Copy link
Contributor

sriniv27 commented Mar 3, 2021

I can pick this up if there isn't any work already happening

@wren
Copy link
Member

wren commented Mar 3, 2021

Yup, go for it.

@sriniv27
Copy link
Contributor

sriniv27 commented Mar 4, 2021

Looks like the invalid linewrap value corresponds to initial_linewrap in FancyExporter::export_entry().

@sriniv27
Copy link
Contributor

sriniv27 commented Mar 4, 2021

Seems like it's sufficient to enforce that the exporter's initial linewrap value is at least 1.

@wren wren removed the 🆕 New! label Mar 6, 2021
@micahellison
Copy link
Member Author

micahellison commented Mar 6, 2021

Seems like it's sufficient to enforce that the exporter's initial linewrap value is at least 1.

I agree, though it should be made clear to the user as to what the actual linewrap config value should be. This is dynamic based on the combination of the date and the time format, so I think the error message should be something like, "Fancy export requires linewrap to be at least X for the selected time format" where X is calculated beforehand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants