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

Colormatching, Markdown, error handling #4

Closed
wants to merge 2 commits into from
Closed

Conversation

itst
Copy link

@itst itst commented Apr 11, 2018

  • Added: colormatching. First attempt to use highlight colors to structure the output. Since highlight colors differ from PDF reader to PDF reader, we try to determine the nearest color of a highlight against a set of reference colors. Blue: Heading level 1. Lilac: Heading Level 2. Green: Marked text. Yellow: Marked bibliographic data. Default id green.
  • Removed: TextWrapper: To output the hierarchie created by colormatching, I implemented markdown.
  • Added: I added some better error handling. I have feeling this is just the tip of the iceberg.

* Added: colormatching. First attempt to use highlight colors to structure the output. Since highlight colors differ from PDF reader to PDF reader, we try to determine the nearest color of a highlight against a set of reference colors. Blue: Heading level 1. Lilac: Heading Level 2. Green: Marked text. Yellow: Marked bibliographic data. Default id green.
* Removed: TextWrapper: To output the hierarchie created by colormatching, I implemented markdown.
* Added: I added some **better** error handling. I have feeling this is just the tip of the iceberg.
Colormatching, Markdown, errorhandling
@0xabu
Copy link
Owner

0xabu commented Apr 11, 2018

Hi,

Thanks for the contribution! Unfortunately I'm not going to merge this:

  • I obviously can't take the German-language strings, unless you want to go to the trouble of introducing a string translation framework.

  • Ultimately, it's the direction of the tool is driven by my main use-case of conference reviews and I'm not convinced by the colour-match functionality. If you made it configurable (under a runtime flag) such that the extra module dependency wasn't needed unless the flag was set then I'd consider it... but I suspect that's more trouble than it's worth to you.

  • Likewise, I like text wrapping :) We probably need a flag to turn it off.

I would be happy to take specific fixes to handle errors in places where you encountered crashes. I know the tool isn't very good at this right now (it has only been tested with PDFs from a handful of sources).

@itst
Copy link
Author

itst commented Apr 11, 2018

Hi,

all fair points! I am sorry for the language strings, simple overlooked them while setting up the merge request.

I will tinker with this thing a bit more, and most importantly test it with more PDFs. My use case is rather long-form PDFs, say textbooks, and I highlight and makes notes for my personal use. I see that this differs from yours. By reviews, you mean copy-editing reviews?

Python isn't my mother tongue, but the config options you mentioned are on my list. As is a templated output.

I will setup a new merge reguest with just the error handling things in the next days.

Thanks, and thank you for writing and sharing this in the first place.

Best
Sascha

@0xabu
Copy link
Owner

0xabu commented Apr 11, 2018

I meant reviews of submissions for academic conferences/journals etc.

Thanks for the separate PR -- I left some review comments there. I'll close this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants