improved trim regex: simpler, safer and more tested

ddnexus · May 24, 2019 · b403a35 · b403a35
1 parent bd3be09
commit b403a35
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 12 deletions.
diff --git a/lib/javascripts/pagy.js b/lib/javascripts/pagy.js
@@ -98,6 +98,6 @@ Pagy.addInputEventListeners = function(input, handler){
                               };
 
 Pagy.trim = function(html, param){
-              var re = new RegExp('[?&]' + param + '=1(?![&])|(?<=[?&])' + param + '=1&');
+              var re = new RegExp('[?&]' + param + '=1\b(?!&)|\b' + param + '=1&');
               return html.replace(re, '');
             };
diff --git a/lib/pagy/extras/trim.rb b/lib/pagy/extras/trim.rb
@@ -10,11 +10,11 @@ module Frontend
 
     alias_method :pagy_link_proc_without_trim, :pagy_link_proc
     def pagy_link_proc_with_trim(pagy, link_extra='')
-      link_proc  = pagy_link_proc_without_trim(pagy, link_extra)
-      page_param = pagy.vars[:page_param]
+      link_proc = pagy_link_proc_without_trim(pagy, link_extra)
+      re = /[?&]#{pagy.vars[:page_param]}=1\b(?!&)|\b#{pagy.vars[:page_param]}=1&/
       lambda do |n, text=n, extra=''|
         link = link_proc.call(n, text, extra)
-        n == 1 ? link.sub(/[?&]#{page_param}=1(?![&])|\b(?<=[?&])#{page_param}=1&/, '') : link
+        n == 1 ? link.sub(re, '') : link
       end
     end
     alias_method :pagy_link_proc, :pagy_link_proc_with_trim

diff --git a/test/pagy/extras/trim_test.rb b/test/pagy/extras/trim_test.rb
@@ -20,14 +20,26 @@
   describe "#pagy_link_proc" do
 
     it 'returns trimmed link' do
-      pagy = Pagy.new(count: 1000)
-      link = view.pagy_link_proc(pagy)
-      link.call(1).must_equal("<a href=\"/foo\"   >1</a>")
-      link.call(10).must_equal("<a href=\"/foo?page=10\"   >10</a>")
-      pagy = Pagy.new(count: 1000, params: {a:3,b:4})
-      link = view.pagy_link_proc(pagy)
-      link.call(1).must_equal("<a href=\"/foo?a=3&b=4\"   >1</a>")
-      link.call(10).must_equal("<a href=\"/foo?page=10&a=3&b=4\"   >10</a>")
+      [ [1,  '?page=1',               ''],                    # only param
+        [1,  '?page=1&b=2',           '?b=2'],                # first param
+        [1,  '?a=1&page=1&b=2',       '?a=1&b=2'],            # middle param
+        [1,  '?a=1&page=1',           '?a=1'],                # last param
+
+        [1,  '?my_page=1&page=1',     '?my_page=1'],          # skip similar first param
+        [1,  '?a=1&my_page=1&page=1', '?a=1&my_page=1'],      # skip similar middle param
+        [1,  '?a=1&page=1&my_page=1', '?a=1&my_page=1'],      # skip similar last param
+
+        [11, '?page=11',              '?page=11'],            # don't trim only param
+        [11, '?page=11&b=2',          '?page=11&b=2'],        # don't trim first param
+        [11, '?a=1&page=11&b=2',      '?a=1&page=11&b=2'],    # don't trim middle param
+        [11, '?a=1&page=11',          '?a=1&page=11']         # don't trim last param
+      ].each do |args|
+        page, generated, trimmed = args
+        view = MockView.new("http://example.com:3000/foo#{generated}")
+        pagy = Pagy.new(count: 1000, page: page)
+        link = view.pagy_link_proc(pagy)
+        link.call(page).must_equal("<a href=\"/foo#{trimmed}\"   >#{page}</a>")
+      end
     end
 
   end