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

Feature: Only 1 print proc (SVG) #24

Merged
merged 2 commits into from
Sep 13, 2022
Merged

Conversation

aruZeta
Copy link
Owner

@aruZeta aruZeta commented Sep 11, 2022

For better readability and ease of feature adding (like I'm planning to add a "margin" field to separate the modules at will, and not the default 0.1 for rounded modules), I think it will be better to just have 1 single printSvg procedure.

Issues:

The proc doesn't know if the user passed a 0 explicitly or not to moRad or alRad, but this should not be an issue at all unless the user wants to have an SVG with individual modules and alignment patterns (I mean them being drawed as <rec> instead of a <path>).

Solution 1: Use Option, problem: I don't like how the user will need to use some() to pass values.

Solution 2 (imo the best): If the user really wants to do what I said before, we could just add a useRect bool field, if true the proc will not care if the specified radius is 0 or whatever.

Comment which solution you like, or add a new one.

Edit: note how this also fixes an issue where you can't set moRad without setting alRad.

@aruZeta
Copy link
Owner Author

aruZeta commented Sep 11, 2022

How it would look with Solution 2:

@@ -99,20 +99,21 @@ func printSvg*(
   alRad: Percentage = 0,
   moRad: Percentage = 0,
   class: string = "qrCode",
-  id: string = ""
+  id: string = "",
+  forceUseRect: bool = false
 ): string =
   result = fmt(svgHeader)
-  if moRad:
+  if moRad or forceUseRect:
     let moRadPx: float32 = 0.4 * moRad / 100
     drawRegionWithoutAlPatterns moduleRect
   else:
     result.add fmt(modulePathStart)
-    if alRad:
+    if alRad or forceUseRect:
       drawRegionWithoutAlPatterns modulePath
     else:
       drawRegion 0'u8, size, 0'u8, size, modulePath
     result.add fmt(modulePathEnd)
-  if alRad:
+  if alRad or forceUseRect:
     let alRadPx: float32 = 3.5 * alRad / 100
     drawRoundedAlignmentPatterns
   result.add svgEnd

With this

let qr = newQR("https://github.com/aruZeta/QRgen")
writeFile(
  "build" / "testingSvg.svg",
  qr.printSvg("#1d2021", "#98971a", forceUseRect = true)
)

Outputs the same as pre- 530e7aa

let qr = newQR("https://github.com/aruZeta/QRgen")
writeFile(
  "build" / "testingSvg.svg",
  qr.printSvg("#1d2021", "#98971a", 0, 0)
)

@aruZeta aruZeta added feature This PR adds a new feature to the next major or minor release. breaking change This PR adds main API breaking changes to the next major release. labels Sep 11, 2022
@aruZeta
Copy link
Owner Author

aruZeta commented Sep 13, 2022

I will go with Solution 2

@aruZeta aruZeta merged commit 2465d86 into develop Sep 13, 2022
@aruZeta aruZeta deleted the feature/only-1-printSvg-proc/1 branch September 13, 2022 14:14
@aruZeta aruZeta mentioned this pull request Sep 15, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This PR adds main API breaking changes to the next major release. feature This PR adds a new feature to the next major or minor release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant