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

added more options to fine tune positioning, omit/change prefix, modify QR code look and visual splitting of the printed number #25

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Unostot
Copy link

@Unostot Unostot commented Jun 4, 2024

This is one of my first tries to contribute something, so it might not be the perfect pull request...help appreciated :)

I Added new options to customize and fine tune the printed labels:

  • --prefix: Prefix in front of digits (default: ASN)
  • --omit-prefix: Do not print prefix in front of digits (just use it in the generated QR code)
  • --x-offset: Horizontal (X) offset in mm (default: 0)
  • --y-offset: Vertical (Y) offset in mm (default: 0)
  • --qr-scale-factor: Scale Factor of QR code (default: 0.9). Makes this option easier to play around until satisfied with the look.
  • --qr-border-size: Size of white border around QR code (default: 4) Makes this option easier to play around until satisfied with the look.
  • --split-at: visually split printed number at given digit position . Multiple timse allowed to add multiple spaces.

x/y offsets of whole label array, omit printing of the label, qr scale factor, qr border size and visual splitting of the printed number
@margau margau mentioned this pull request Jul 13, 2024
@vicegold
Copy link

Is this blocked by anything? Would love to use a different prefix :)

Copy link

@alexanderbittner alexanderbittner left a comment

Choose a reason for hiding this comment

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

After reviewing the code and running this locally to try out the new arguments, this appears to work well. No idea why the CI doesn't work on this PR, as posted above.
Let's hope a manual review helps!

Copy link
Contributor

@promasu promasu left a comment

Choose a reason for hiding this comment

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

The commit changes the file mode from 10644 to 10755. This should be reverted.

@Unostot
Copy link
Author

Unostot commented Jan 26, 2025

changed it back, not sure why it happend in the first place :)

@Unostot Unostot requested a review from promasu January 26, 2025 17:55
paperless_asn_qr_codes/main.py Outdated Show resolved Hide resolved
paperless_asn_qr_codes/main.py Outdated Show resolved Hide resolved
Unostot and others added 2 commits January 26, 2025 19:37
corrected typo

Co-authored-by: Adrian Nöthlich <[email protected]>
removed print statement

Co-authored-by: Adrian Nöthlich <[email protected]>
@Unostot Unostot requested a review from promasu January 26, 2025 18:39
Copy link
Contributor

@promasu promasu left a comment

Choose a reason for hiding this comment

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

You are to quick :D
Please add the new parameters to the README as well so it's up to date

@promasu promasu requested a review from margau January 26, 2025 18:40
improved vertical alignment of scaled QR code (should now be centered somehow on the vertical axis)
@Unostot
Copy link
Author

Unostot commented Jan 26, 2025

You are to quick :D Please add the new parameters to the README as well so it's up to date

I've added the options to the Readme, and also improved a bit the vertical alignment. This should help whith qr-scale-factors != 0.9 to have it centered along the vertical axis.

@Unostot Unostot requested a review from promasu January 26, 2025 19:17
Copy link
Collaborator

@margau margau left a comment

Choose a reason for hiding this comment

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

I'm not too sure about this now.
Feature wise it is surely great, but at the moment, our main.py is a big mess without proper classes.

I'm therefore right now not too happy about adding additional "globals" and "non-class-based abstraction", as it becomes more difficult to read and maintain.

@Unostot
Copy link
Author

Unostot commented Jan 26, 2025

I'm not too sure about this now. Feature wise it is surely great, but at the moment, our main.py is a big mess without proper classes.

I'm therefore right now not too happy about adding additional "globals" and "non-class-based abstraction", as it becomes more difficult to read and maintain.

is there alread any plan how this should be changed in the future? i.e. do you have a rough idea how to class everything and such, so it becomes more separated and easier to maintain?

I would be willing to help, if I know how it will be changed, and maybe I'm even able to convert my changes to the new design...

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.

5 participants