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

Pretty print journal entries #692

Merged
merged 43 commits into from
Nov 19, 2019
Merged

Pretty print journal entries #692

merged 43 commits into from
Nov 19, 2019

Conversation

alichtman
Copy link
Contributor

@alichtman alichtman commented Oct 25, 2019

Changes appearance of all jrnl viewing commands, such as $ jrnl --short and
$ jrnl -n {NUM}.

Fix #508, #717

Changes appearance of all jrnl viewing commands, such as $ jrnl --short and
$ jrnl -n {NUM}.

Fix #508
@alichtman alichtman changed the title Pretty print journal entry titles and dates. Pretty print journal entry titles and dates Oct 25, 2019
@alichtman
Copy link
Contributor Author

alichtman commented Oct 25, 2019

This feature has some slight bugs that I'm still working out.

  1. I'm not sure where the extra newline between the title and the body is coming from. I sort of want to consider this a feature and not a bug. I like the way it looks, actually. Edit: Fixed

image

  1. A bunch of feature tests are failing because the output is being produced with certain ANSI escape characters and the expected output doesn't contain them. Not sure exactly how to fix this one yet, but I've been playing around with some stuff in the features files. For example, Edit: Fixed
 Scenario: --short displays the short version of entries (only the title)  # features/core.feature:54
    Given we use the config "basic.yaml"                                    # features/steps/core.py:66 0.000s
    When we run "jrnl -on 2013-06-10 --short"                               # features/steps/core.py:90 0.004s
    Then the output should be "2013-06-10 15:40 Life is good."              # features/steps/core.py:169 0.000s
      Assertion Failed: ['2013-06-10 15:40 Life is good.', '\x1b[91m2013-06-10 15:40\x1b[0m \x1b[1mLife is good.\x1b[0m']
      Captured stdout:
      2013-06-10 15:40 Life is good.

@alichtman
Copy link
Contributor Author

alichtman commented Oct 26, 2019

The last failing test case is exporting with custom templates. This is also failing for me locally on master (but not on Travis) so it's possible that it's not my PR causing this.

Yep, just forgot to commit the file that contained the new dependency.

This seems ready to be merged to me, unless you have anything you'd like changed.

@pspeter
Copy link
Contributor

pspeter commented Nov 1, 2019

I'm not the biggest fan of the color choices to be honest. Why not let the user define the colors (separately for date, title and body) in the config?

@alichtman
Copy link
Contributor Author

Why not let the user define the colors (separately for date, title and body) in the config?

Good call. Do you think it's better to have them set colors like red, blue, green, ... or ANSI escapes?

@pspeter
Copy link
Contributor

pspeter commented Nov 3, 2019

Definitely colors. Not sure what the ideal way to map them to escapes is though. We may want to look into moving away from using the raw escapes all together and towards a package like colorama.

features/steps/core.py Outdated Show resolved Hide resolved
- Replaced raw escapes with colorama
- Added colors key to config
- Add checks for validity of color values
@alichtman
Copy link
Contributor Author

alichtman commented Nov 6, 2019

I should add two tests:

  1. Identifying invalid color configs
  2. Upgrading the config from no colors -> colors

(Done)

jrnl/util.py Outdated Show resolved Hide resolved
@alichtman
Copy link
Contributor Author

@wren I believe this is ready for review.

@wren
Copy link
Member

wren commented Nov 6, 2019

@alichtman Great! I'll review this weekend.

One thing from first glance, though: Is there an option to disable color? I know a lot of people pipe output to various scripts, and the escape sequences can complicate that.

@wren wren added the enhancement New feature or request label Nov 6, 2019
@alichtman
Copy link
Contributor Author

Yep, I can add that.

jrnl/util.py Outdated Show resolved Hide resolved
jrnl/util.py Outdated Show resolved Hide resolved
jrnl/install.py Outdated Show resolved Hide resolved
@alichtman
Copy link
Contributor Author

alichtman commented Nov 6, 2019

I've found one bug in the config updating algorithm that will need to be addressed. It only looks at top-level keys, and not nested keys.

https://stackoverflow.com/a/27266178

A solution like this will be needed if the subkeys of colors will ever be updated...

I'll open a separate issue. (#714)

@alichtman
Copy link
Contributor Author

alichtman commented Nov 6, 2019

image

So here's an interesting bug...

I understand why the body drops all colorama formatting after the tag (due to the colorama.Style.RESET_ALL in colorize()), but I can't figure out why the title doesn't have the same problem. I've pushed the most recent version if you'd like to play around with it, @peter-schmidbauer.

EDIT: It seems to break like I was expecting it to in PyCharm, just not in iTerm2.

image

Seems like in order to fix this, we'd need to do the tag parsing before the colorization, which would mean either doing it all in the entry.pprint() method or returning an unmerged entry to the journal.pprint() method. As it is now, there's no way to determine whether or not the tag is in the title or the body.

@pspeter
Copy link
Contributor

pspeter commented Nov 6, 2019

By the way, I think stripping the color from all outputs during tests is not ideal. I'd instead have all previous tests use no color, and add a new configuration with colors and some more specific tests. It's a new feature and deserves its own tests imho.

@alichtman
Copy link
Contributor Author

I tried for a few hours to figure out how to get behave to work with ANSI escapes and made no progress. I agree with you, but I don't think it's possible.

@wren
Copy link
Member

wren commented Nov 12, 2019

Hey, we just merged a bunch of stuff into master. This just needs a quick rebase on the new master, but then should be ready to go. Thanks, and sorry!

@alichtman
Copy link
Contributor Author

alichtman commented Nov 12, 2019

Disclaimer: I did that merge in the GitHub UI. I'm pretty sure it's done correctly but we shall see...

Yup, all tests pass. Good to go here.

@alichtman alichtman changed the title Pretty print journal entry titles and dates Pretty print journal entries Nov 12, 2019
@alichtman
Copy link
Contributor Author

alichtman commented Nov 12, 2019

Resolved merge conflict in poetry.lock.

@alichtman
Copy link
Contributor Author

@wren Any update on this?

@wren
Copy link
Member

wren commented Nov 15, 2019

@alichtman I have a full jrnl work day scheduled for Saturday, so I'll be able to go fully through this then. Lots a big one, so I want to make sure to put it through its paces.

Copy link
Contributor

@pspeter pspeter left a comment

Choose a reason for hiding this comment

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

Be careful when merging stuff!

jrnl/util.py Outdated Show resolved Hide resolved
jrnl/Entry.py Outdated Show resolved Hide resolved
features/steps/core.py Outdated Show resolved Hide resolved
@wren
Copy link
Member

wren commented Nov 17, 2019

This is looking good so far. I didn't quite get this one finished, but I'll wrap up on Monday night. Thanks for your patience!

@wren wren changed the base branch from master to v2.2 November 19, 2019 03:52
Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for all the work!

@wren wren merged commit 9ca09c5 into jrnl-org:v2.2 Nov 19, 2019
@alichtman alichtman deleted the pretty-print-entries branch November 19, 2019 12:22
@alichtman alichtman mentioned this pull request Nov 20, 2019
@alichtman alichtman restored the pretty-print-entries branch November 20, 2019 15:17
@alichtman alichtman deleted the pretty-print-entries branch November 20, 2019 15:21
@wren wren removed this from the v2.2 milestone Jan 26, 2020
wren pushed a commit that referenced this pull request Feb 29, 2020
* Pretty print journal entry titles and dates.

Changes appearance of all jrnl viewing commands, such as $ jrnl --short and
$ jrnl -n {NUM}.

Fix #508

* Removed extra newline at end of title

* Use ansiwrap to properly wrap strings with ANSI escapes

* Add ansiwrap to pyproject.toml

* Allow configuration of colors

- Replaced raw escapes with colorama
- Added colors key to config
- Add checks for validity of color values

* Add color configuration documentation

* Fix broken tests due to config change

* Add tests for colors in configs

- Identifying invalid color configs
- Upgrading config from no colors -> colors

* Add colorama dependency for all platforms

* Allow users to disable colorization of output

* Update poetry.lock

* Add tag and body color customization options

* Fix colorization of tags in title and body

* Updated tests to use no color by default

* Change pass to continue in verify_config()

* Better style in Entry.py

* Reduce code duplication for tag highlighting

- Breaks "unreadable date" regression test for unknown reason

* Properly colorize tags and print body

* Reformatting and clean up

* Replace list comprehension with generator

* Handle invalid colors by not using a color

* Process ANSI escapes properly with behave

* Fixed the 'spaces after tags directly next to punctuation' bug

Broke processing of tags next to any punctuation at all

* Closer to working tag colorization but not perfect

* Add tests printing for multiline journals

Fix #717

* Correctly indent first line of multiline entry

* Add test for multiline entries with tags

* Remove redundant UNICODE flag

* Progress towards proper tag colorization and body formatting

* Fix newline colorization bug

Debug code left intact since there are more bugs to fix :/

* And now the space just ends up before the tag instead of after it

* Fix assertion syntax warning

* Moved tag test to tagging.feature file

* Strip out debug code and clean up

* Bold datetimes in title

* Bold all titles

Fix #720

* Remove PY2 and PY3 constants

* Fix regression in features/steps/core.py

* Fix tag_regex

* Remove redundant re.UNICODE flag

* Remove extraneous code
wren pushed a commit that referenced this pull request Feb 29, 2020
* Pretty print journal entry titles and dates.

Changes appearance of all jrnl viewing commands, such as $ jrnl --short and
$ jrnl -n {NUM}.

Fix #508

* Removed extra newline at end of title

* Use ansiwrap to properly wrap strings with ANSI escapes

* Add ansiwrap to pyproject.toml

* Allow configuration of colors

- Replaced raw escapes with colorama
- Added colors key to config
- Add checks for validity of color values

* Add color configuration documentation

* Fix broken tests due to config change

* Add tests for colors in configs

- Identifying invalid color configs
- Upgrading config from no colors -> colors

* Add colorama dependency for all platforms

* Allow users to disable colorization of output

* Update poetry.lock

* Add tag and body color customization options

* Fix colorization of tags in title and body

* Updated tests to use no color by default

* Change pass to continue in verify_config()

* Better style in Entry.py

* Reduce code duplication for tag highlighting

- Breaks "unreadable date" regression test for unknown reason

* Properly colorize tags and print body

* Reformatting and clean up

* Replace list comprehension with generator

* Handle invalid colors by not using a color

* Process ANSI escapes properly with behave

* Fixed the 'spaces after tags directly next to punctuation' bug

Broke processing of tags next to any punctuation at all

* Closer to working tag colorization but not perfect

* Add tests printing for multiline journals

Fix #717

* Correctly indent first line of multiline entry

* Add test for multiline entries with tags

* Remove redundant UNICODE flag

* Progress towards proper tag colorization and body formatting

* Fix newline colorization bug

Debug code left intact since there are more bugs to fix :/

* And now the space just ends up before the tag instead of after it

* Fix assertion syntax warning

* Moved tag test to tagging.feature file

* Strip out debug code and clean up

* Bold datetimes in title

* Bold all titles

Fix #720

* Remove PY2 and PY3 constants

* Fix regression in features/steps/core.py

* Fix tag_regex

* Remove redundant re.UNICODE flag

* Remove extraneous code
wren pushed a commit that referenced this pull request Apr 18, 2020
* Pretty print journal entry titles and dates.

Changes appearance of all jrnl viewing commands, such as $ jrnl --short and
$ jrnl -n {NUM}.

Fix #508

* Removed extra newline at end of title

* Use ansiwrap to properly wrap strings with ANSI escapes

* Add ansiwrap to pyproject.toml

* Allow configuration of colors

- Replaced raw escapes with colorama
- Added colors key to config
- Add checks for validity of color values

* Add color configuration documentation

* Fix broken tests due to config change

* Add tests for colors in configs

- Identifying invalid color configs
- Upgrading config from no colors -> colors

* Add colorama dependency for all platforms

* Allow users to disable colorization of output

* Update poetry.lock

* Add tag and body color customization options

* Fix colorization of tags in title and body

* Updated tests to use no color by default

* Change pass to continue in verify_config()

* Better style in Entry.py

* Reduce code duplication for tag highlighting

- Breaks "unreadable date" regression test for unknown reason

* Properly colorize tags and print body

* Reformatting and clean up

* Replace list comprehension with generator

* Handle invalid colors by not using a color

* Process ANSI escapes properly with behave

* Fixed the 'spaces after tags directly next to punctuation' bug

Broke processing of tags next to any punctuation at all

* Closer to working tag colorization but not perfect

* Add tests printing for multiline journals

Fix #717

* Correctly indent first line of multiline entry

* Add test for multiline entries with tags

* Remove redundant UNICODE flag

* Progress towards proper tag colorization and body formatting

* Fix newline colorization bug

Debug code left intact since there are more bugs to fix :/

* And now the space just ends up before the tag instead of after it

* Fix assertion syntax warning

* Moved tag test to tagging.feature file

* Strip out debug code and clean up

* Bold datetimes in title

* Bold all titles

Fix #720

* Remove PY2 and PY3 constants

* Fix regression in features/steps/core.py

* Fix tag_regex

* Remove redundant re.UNICODE flag

* Remove extraneous code
@lock
Copy link

lock bot commented May 20, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the 🔒 Outdated label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request 🔒 Outdated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants