andymass / vim-matchup

vim match-up: even better % :facepunch: navigate and highlight matching words :facepunch: modern matchit and matchparen. Supports both vim and neovim + tree-sitter.
https://www.vim.org/scripts/script.php?script_id=5624
MIT License
1.69k stars 71 forks source link

[Bug] SQL keywords matching problem #3

Open letientai299 opened 6 years ago

letientai299 commented 6 years ago

Explain the issue

See the files content below.

  1. Steps to reproduce

    • Start vim with › HOME=$PWD MYVIMRC=$PWD/matchup_vimrc vim test.sql
    • Put cursor as drop or function in line 1, press %.
  2. Expected behavior

    • Nothing
  3. Observed behavior

    • Cusor jump to line 8 (return;).
    • Try again with the second function (line 3), cursor jump to returns at line 4.

Minimal working example

Please provide a minimal working example, e.g.,

-- file test.sql
drop function if exists dummy;

create or replace function dummy()
returns setof text as 
$$
begin
    return next 'something';
    return;
end
$$ language 'plpgsql';

Minimal vimrc file

" file matchup_vimrc
set nocompatible
let &rtp  = '~/.vim-plugged/vim-matchup/,' . &rtp
filetype plugin indent on
syntax enable
andymass commented 6 years ago

The behavior in matchit is the same. I think it is due to a problem with b:match_words set by the ftplugin/sql.vim (maintained by David Fishburn). This example still seems challenging.

First, a question: if the cursor is on line 3, over create or replace function, where should % go? And after pressing % again? And after pressing it once more?

Cycling between create or replace function (line 3) and returns (line 4) would be easy. Cycling between the two return statements will be more difficult, because there is no "end of function" marker to match the create or replace function. So, should return next/return; belong to the begin and end?

letientai299 commented 6 years ago

First, a question: if the cursor is on line 3, over create or replace function, where should % go? And after pressing % again? And after pressing it once more?

In that case, I think cycling between create or replace and the return next/return is the expected behavior. The returns usually not far from create or replace and it's is not the end of function also. If we need to a mark for "end of function", I think it would be return;.

andymass commented 6 years ago

This may work for you. Place it in your vimrc or wherever it can be run before opening files of type sql.

" test/issues/3/hotfix.vim
function! SQLHotFix()
    call matchup#util#patch_match_words(
        \ '\%(\<create\s\+\%(or\s\+replace\s\+\)\?\)\?'
        \ . '\%(function\|procedure\|event\)'
        \ . ':\<returns\?\>',
        \ '\%(\<create\s\+\%(or\s\+replace\s\+\)\?\)'
        \ . '\%(function\|procedure\|event\)'
        \ . ':\<returns\>'
        \ . ':\<return\s\+\%(next\|query\)\>'
        \ . ':\<return\>\ze\%(\s\+next\)\@!\%(.\|$\)'
        \)
endfunction

let g:matchup_hotfix_sql = 'SQLHotFix'

Note that you can remove the line \ . ':\<returns\>' (line 9 here) if you prefer to not stop at the first returns. If it makes sense, it may be possible to request a change from the maintainer of vim's sql ftplugin. I am not comfortable enough with all the various dialects of SQL to know whether this patch works for all cases or just your test example.

letientai299 commented 6 years ago

Thanks for the hot fix. It works with my simple example, but it won't work with bigger function. e.g.

drop function if exists another_function;
create or replace function another_function()
returns setof text as
$$
begin
    if 1=0 then 
        return 'impossible!';
    end if;
  return next 'something';
  return;
end
$$ language 'plpgsql';

Anyway, I can live with the current state of matchup 😄

andymass commented 6 years ago

Glad it's mostly working for you :smiley:.

You are right, I should have thought of that example. Anyway, I cannot think of a way to handle this correctly with the current implementation. The reason is there needs to be a definite end word. Sequences of matching words must come as begin:middle*:end, otherwise there is no way to know where to stop searching. So, I am considering this an enhancement and it might take a while to get to.