Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to disable inline math #378

Merged
merged 8 commits into from
Jan 10, 2023

Conversation

tnyeanderson
Copy link
Contributor

@tnyeanderson tnyeanderson commented Jun 8, 2022

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 :)

Copy link
Member

@alerque alerque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good so far. I'm a little puzzled myself why the other method didn't work from you. Why don't you leave this commit as is and post another commit on top of it to this PR that shows your best stab at the expected way to make this work and we'll look into it from there and see if we can make out why id didn't go as planned?

@tnyeanderson
Copy link
Contributor Author

@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
Copy link
Member

alerque commented Jul 22, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor Author

tnyeanderson commented Jul 31, 2022

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
Copy link
Contributor Author

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

@tnyeanderson
Copy link
Contributor Author

@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
Copy link
Member

alerque commented Sep 13, 2022

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
Copy link
Contributor Author

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 tnyeanderson marked this pull request as ready for review December 7, 2022 06:24
@tnyeanderson
Copy link
Contributor Author

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 tnyeanderson marked this pull request as draft December 7, 2022 06:34
@tnyeanderson
Copy link
Contributor Author

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 tnyeanderson marked this pull request as ready for review December 7, 2022 07:12
@tnyeanderson
Copy link
Contributor Author

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 alerque merged commit 4268535 into vim-pandoc:master Jan 10, 2023
@alerque
Copy link
Member

alerque commented Jan 10, 2023

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add option to disable math highlighting
2 participants