vim-pandoc / vim-pandoc-syntax

pandoc markdown syntax, to be installed alongside vim-pandoc
MIT License
425 stars 61 forks source link

Add the ability to disable inline math #378

Closed tnyeanderson closed 1 year ago

tnyeanderson commented 2 years ago

Fixes: #196

Adds an option for inlinemath to the blacklist variable that controls whether inline math will be concealed or not.

At the moment it works but is untested and almost definitely not the right way. I feel like I should be using WithConceal like the other blacklist items, but when I tried to plug the two syn region lines into WithConceal it broke. So for now I am checking the index() of the blacklist array variable directly.

Could anyone more familiar with the code point me in the right direction here? This is my first time contributing to a vim plugin :)

tnyeanderson commented 1 year ago

@alerque I've added another commit to try to use WithConceal. Unfortunately when trying to do it that way, it seems to have no effect (inlinemath objects are still formatted despite being blacklisted). I basically put the whole syn region ... line as the second argument (rule) after looking at how others were implemented, but I'm sure that's not correct (otherwise it would work!)

Hopefully you can point me in the right direction!

alerque commented 1 year ago

Having given this a look over but not having gotten it to work yet, I'd take a stab and guess the issue has to do with escaping the regex inside the single quotes. Did you try it with some easy to debug keyword to match the start/end to that doesn't need any sort of regex, like DEBUGSTARTMATH and DEBUGENDMATH just to see if the rest is working, then figure out how to escape the actual expression from there?

tnyeanderson commented 1 year ago

Did you try it with some easy to debug keyword to match the start/end to that doesn't need any sort of regex, like DEBUGSTARTMATH and DEBUGENDMATH

Good idea, I'll give that a shot this weekend!

tnyeanderson commented 1 year ago

Okay, with the latest commmit a92bba7 I've added an entry to aid with debugging. Here are my findings, formatted as a test markdown file to demonstrate the issue:

# Testing inlinemath for vim-pandoc-syntax

## Working

Debug entry: DEBUGSTARTMATHthistextshouldbeconcealedDEBUGENDMATH

> The above text conceals properly, and is ignored when blacklisted (as expected).

## Not working

Using dollar signs: $thistextcannotbeignored$

Using parenthesis: \(thistextcannotbeignored\)

> The above text conceals regardless of whether it is blacklisted

To reproduce:

  1. Make sure you are using commit a92bba7
  2. Make sure that inlinemathdebug and inlinemath are not in the blacklist array
  3. Open the markdown file above. The debug entry will be hidden completely, and the dollar signs/parenthesis entries will be highlighted (both are being concealed properly)
  4. Add inlinemathdebug and inlinemath to the blacklist array
  5. Reopen the above markdown file. The debug entry will not be concealed anymore (as expected), but the dollar signs/parenthesis entries will still be highlighted/concealed (not desired behavior)

NOTE: If lines 264 and 265 are commented, the dollar signs/parenthesis entries will no longer be concealed, so it does appear that those lines are controlling it!

I really wish I knew what was going on here, maybe it's something stupid and I just need a sanity check :)

tnyeanderson commented 1 year ago

@alerque any ideas? I'm at a loss here...

tnyeanderson commented 1 year ago

@alerque don't mean to bother as I'm sure you're very busy... just want to ping you to make sure this doesn't fall off the radar :)

alerque commented 1 year ago

Sorry, it hasn't fallen off the radar completely but I've been pretty swamped and will be traveling for another month. I haven't had a chance to dig into this.

Did you check the difference between single and double quoting strings? In Vimscript how strings are quoted affects how regular expressions are parsed.

tnyeanderson commented 1 year ago

Fixed the problem! It has to do with the logic in the WithConceal function:

" WithConceal {{{2
function! s:WithConceal(rule_group, rule, conceal_rule)
    let l:rule_tail = ''
    if g:pandoc#syntax#conceal#use != 0
        if index(g:pandoc#syntax#conceal#blacklist, a:rule_group) == -1
            let l:rule_tail = ' ' . a:conceal_rule
        endif
    endif
    execute a:rule . l:rule_tail
endfunction
" }}}2

As seen here, the execute a:rule . l:rule_tail always runs, it just leaves out the conceal_rule if the rule_group is blacklisted. I assume this is so concealing of parts can be turned on manually after opening the file, even if it is blacklisted when the file is first opened.

Of course, not understanding this critical distinction, I had a contains=@LATEX parameter in my second argument, meaning it was always getting called. Moving it to the third argument fixes the issue.

tnyeanderson commented 1 year ago

ugh. nevermind. now it's still breaking things. I think it has to do with the fact that the syn call is being made at all, it's preventing other conceals from happening after a single $. Putting back in draft to investigate. Sincere apologies for the noise

tnyeanderson commented 1 year ago

Unfortunately, with what I've learned in my last few tests, I think that WithConceal cannot be used here. I've reverted to the original solution (a standard conditional) and added a note explaining the situation. I think this is the best, clearest way to solve the problem, so I'm marking this ready for review (again, very sorry for the noise).

Not sure if y'all squash. If so I'll keep history as-is. Otherwise, let me know if you want me to rebase and clean up before it gets merged. Thanks everyone!!

tnyeanderson commented 1 year ago

Hey @alerque I'm sure you're very very busy, just want to make sure this doesn't fall off the radar will all the new year craziness. Hope you can take a look some time soon. Thanks so much!!

alerque commented 1 year ago

Sorry, yes this had fallen off the radar. It's still probably in notifications but I've been pulling from the top at a trickle...