dunst-project / dunst

Lightweight and customizable notification daemon
https://dunst-project.org
Other
4.44k stars 338 forks source link

Corners patch part 1 #1227

Closed bynect closed 6 months ago

bynect commented 8 months ago

I divided the patch in #1213 to simplify things a bit. This first part only deals with the rendering of the corners. The second part (which I will add soon) adds support for the customization option to change corner profile. Anyway, refer to #1213 for any information on the changes.

bynect commented 8 months ago

Should I close the other?

fwsmit commented 8 months ago

Should I close the other?

No that's not neccesary

codecov-commenter commented 8 months ago

Codecov Report

Merging #1227 (a697e8c) into master (19865c0) will decrease coverage by 0.72%. Report is 22 commits behind head on master. The diff coverage is 20.37%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master    #1227      +/-   ##
==========================================
- Coverage   66.03%   65.31%   -0.72%     
==========================================
  Files          46       46              
  Lines        7595     7637      +42     
==========================================
- Hits         5015     4988      -27     
- Misses       2580     2649      +69     
Flag Coverage Δ
unittests 65.31% <20.37%> (-0.72%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/x11/x.c 0.00% <ø> (ø)
test/draw.c 98.87% <100.00%> (+0.01%) :arrow_up:
src/draw.c 42.48% <15.68%> (-8.88%) :arrow_down:

... and 2 files with indirect coverage changes

:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

fwsmit commented 6 months ago

Sorry that I've taken so long to review this. This PR still contains more than just the fix for the progress bar rendering, but I'll try to roll with it. Could you explain how the bug happened in the old progress bar implementation? Good testing is nice, but it's also good to know what went wrong.

fwsmit commented 6 months ago

The code does look good, but I'll have to take a closer later.

bynect commented 6 months ago

Sorry that I've taken so long to review this. This PR still contains more than just the fix for the progress bar rendering, but I'll try to roll with it. Could you explain how the bug happened in the old progress bar implementation? Good testing is nice, but it's also good to know what went wrong.

To be fair, the only thing that is not about corners is the bar adjustment, which is minimal and I already commented in the original pr. (It just deals with either a pango antialiasing/number rounding issue that makes the lines slightly skewed)

When you merge this I can make the part 2 of the pr for costumizing corners

bynect commented 6 months ago

did you manage to see it?

The code does look good, but I'll have to take a closer later.

fwsmit commented 6 months ago

Thanks for the poke. The code looks good, I haven't found any issues with it. Thanks for your patience and working on this fix. I will go ahead and merge this. I look forward to the V2 patch!

bynect commented 6 months ago

Thanks. I'll prepare the patch as soon as I have a little spare time since the code is already there. Sometime next week at most