Bug 12792 - C4::Reserves breaks my vim syntax color
Summary: C4::Reserves breaks my vim syntax color
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-20 11:40 UTC by Jonathan Druart
Modified: 2016-06-21 21:36 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 12792: C4::Reserves breaks my vim syntax color (4.91 KB, patch)
2014-08-20 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12792: C4::Reserves breaks my vim syntax color (5.06 KB, patch)
2014-11-05 17:53 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 12792: C4::Reserves breaks my vim syntax color (5.00 KB, patch)
2014-11-06 21:46 UTC, M. Tompsett
Details | Diff | Splinter Review
[SIGNED OFF] Bug 12792: C4::Reserves breaks my vim syntax color (5.11 KB, patch)
2014-11-06 21:55 UTC, M. Tompsett
Details | Diff | Splinter Review
[PASSED QA] Bug 12792: C4::Reserves breaks my vim syntax color (5.22 KB, patch)
2014-11-08 06:25 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 M. 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 M. 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 M. 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 M. Tompsett 2014-11-06 21:40:02 UTC
Bug 11126 is what is causing the conflict.
Comment 10 M. Tompsett 2014-11-06 21:46:18 UTC Comment hidden (obsolete)
Comment 11 M. Tompsett 2014-11-06 21:55:57 UTC Comment hidden (obsolete)
Comment 12 M. 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!