Bug 12792

Summary: C4::Reserves breaks my vim syntax color
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: katrin.fischer, m.de.rooy, mtompset, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 12792: C4::Reserves breaks my vim syntax color
Bug 12792: C4::Reserves breaks my vim syntax color
Bug 12792: C4::Reserves breaks my vim syntax color
[SIGNED OFF] Bug 12792: C4::Reserves breaks my vim syntax color
[PASSED QA] Bug 12792: C4::Reserves breaks my vim syntax color

Description Jonathan Druart 2014-08-20 11:40:47 UTC
[I am sorry for this...]

C4/Reserves.pm is unreadable with my vim configuration.
It appears I am the only one having this problem.
For an incomprehensible reason, a string constructs with
  qq/my string/;
completely breaks the syntax color for all the rest of the file (~2300l).
If I replace it with
  qq{my string};
all is fine!
Comment 1 Jonathan Druart 2014-08-20 11:44:05 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2014-08-20 11:46:05 UTC
Diving into the wonderful world of vim configuration was no option? :)
Comment 3 Jonathan Druart 2014-08-20 12:04:31 UTC
I didn't find anything.

I know this patch is quite egoist and I don't want to impose something.
If someone does not agree with it, I can close this bug report.
But the patch is trivial...
Comment 4 Mark Tompsett 2014-11-05 07:34:27 UTC
I don't mind this patch myself. But sorry, you modify three functions: AddReserve, GetReserveFee, and _Findgroupreserve. There are only tests for AddReserve in our current test suite. You need to improve test coverage if you want your tweaks in GetReserveFee and _Findgroupreserve to pass QA properly, in my understanding.
Comment 5 Jonathan Druart 2014-11-05 10:03:57 UTC
I am not sure to understand what I could test here...
Comment 6 Mark Tompsett 2014-11-05 17:51:11 UTC
(In reply to Jonathan Druart from comment #5)
> I am not sure to understand what I could test here...

Oops. Nevermind. Turns out the t/db_dependent/Reserves.t triggers them all.
Sign off coming up. :)
Comment 7 Mark Tompsett 2014-11-05 17:53:25 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2014-11-06 21:01:24 UTC
Jonathan, the patch doesn't apply cleanly - can you take a look?
Comment 9 Mark Tompsett 2014-11-06 21:40:02 UTC
Bug 11126 is what is causing the conflict.
Comment 10 Mark Tompsett 2014-11-06 21:46:18 UTC Comment hidden (obsolete)
Comment 11 Mark Tompsett 2014-11-06 21:55:57 UTC Comment hidden (obsolete)
Comment 12 Mark Tompsett 2014-11-06 21:59:32 UTC
(In reply to M. Tompsett from comment #11)
> The prove currently fails on test 8, but the other succeeding
> tests prove that this change is fine.

Oops. Test 11. Still, the change is good and worthy of QA.
Comment 13 Jonathan Druart 2014-11-07 10:59:38 UTC
(In reply to M. Tompsett from comment #12)
> (In reply to M. Tompsett from comment #11)
> > The prove currently fails on test 8, but the other succeeding
> > tests prove that this change is fine.
> 
> Oops. Test 11. Still, the change is good and worthy of QA.

Looks good here:

prove t/db_dependent/Reserves.t
t/db_dependent/Reserves.t .. 1/50 Subroutine C4::Context::userenv redefined at t/db_dependent/Reserves.t line 42.
C4::Context->userenv not defined! at C4/Branch.pm line 143.
C4::Context->userenv not defined! at C4/Items.pm line 2305.
t/db_dependent/Reserves.t .. ok     
All tests successful.
Files=1, Tests=50,  2 wallclock secs ( 0.02 usr  0.00 sys +  1.60 cusr  0.04 csys =  1.66 CPU)
Result: PASS
Comment 14 Katrin Fischer 2014-11-08 06:25:44 UTC
Created attachment 33390 [details] [review]
[PASSED QA] Bug 12792: C4::Reserves breaks my vim syntax color

C4/Reserves.pm is unreadable with my vim configuration.
It appears I am the only one having this problem.
For an incomprehensible reason, a string constructs with
  qq/my string/;
completely breaks the syntax color for all the rest of the file (~2300l).
If I replace it with
  qq{my string};
all is fine!

Test plan:
launch
  git show HEAD
and verify this patch won't break anything.

Additionally, prove t/db_dependent/Reserves.t
This will trigger the three functions that were modified.
The prove currently fails on test 8, but the other succeeding
tests prove that this change is fine.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests pass on my installation.
No problems found.
Comment 15 Tomás Cohen Arazi 2015-01-14 15:47:01 UTC
Patch pushed to master.

Thanks Jonathan!