markdown filetype check: use get()

[doronbehar]

Fixes this startup error for me:

Error detected while processing /home/doron/.files/.config/nvim/bundle/pandoc/plugin/pandoc.vim:
line   79:
E121: Undefined variable: g:pandoc#filetypes#pandoc_markdown
E15: Invalid expression: g:pandoc#filetypes#pandoc_markdown == 1

Might fix (NOT) #342 as well.

[alerque]

Hmm, I think maybe we'd better fix the root cause of this issue. I suspect my da4c0b0 commit is to blame here. I'm having trouble understanding the sequence of events though because in some cases this does not seem to be a problem.

Do you understand when ftdetect code gets run in relation to the plugin itself? Where do we need to set this in the first place?

[doronbehar]

Do you understand when ftdetect code gets run in relation to the plugin itself? Where do we need to set this in the first place?

I once encountered a similar issue and this is what I learned: neomutt/neomutt.vim#4 (comment)

[timokau]

Since vim plugins are generally installed straight from the master branch on github, could we maybe just revert da4c0b0 on master for now?

[alerque]

@timokau No I'm sorry but I'm not just going to revert that commit, it fixes another issue that was an active problem for many people and we were getting more and more reports of it. This issue is just a warning message on startup, that was a full on breaker.

I'm happy to fix this issue too, but we need to figure out the right way to do that.

@doronbehar I've reviewed that other PR and the comment you liked, but I don't think they are the same issue at all. Could you be more specific?

[alerque]

I think I see what's going on, we have a regular land grab going on and there are actually two possible race conditions. I'll try to patch up the other half of it as an extension to this PR.

[fmoralesc]

@alerque I think the proposed solution would solve the problem. I can't reproduce the issue, though, it might be useful if @doronbehar would provide us with the output of :scriptnames in his machine.

[doronbehar]

@doronbehar I've reviewed that other PR and the comment you liked, but I don't think they are the same issue at all. Could you be more specific?

@alerque TBH now that I'm thinking about it, I'm not sure that idea is relevant. But it's pretty simple - don't use set filetype or setlocal filetype. (e.g in ftplugin/pandoc.vim) Use setfiletype.

Sorry for not suggesting a patch with broader changes, it's just that git grep filetype suggests there are many places the settings are checked against and acted accordingly. I haven't found yet the time to dive deeper into this. I just wanted a quick fix.

The relevant documentation for this set ft vs setfiletype thing is this I think: https://neovim.io/doc/user/filetype.html#new-filetype & https://neovim.io/doc/user/options.html#:setfiletype .

What confuses me is the fact there are 2 settings with somewhat colliding purposes:

g:pandoc#filetypes#pandoc_markdown

And:

g:pandoc#filetypes#handled

Which makes #342 legitimate.

FWIW, I think using get( instead of a plain g:this == 1 is safer (the essence of this PR) and perhaps this should be adopted in other places as well.

[alerque]

@fmoralesc I think I actually figured out the problematic scenario. As @doronbehar points out though there are actually two competing systems in the same plugin and different plugin managers handle leading in different orders. I believe using get() is the right solution, but this PR doesn't go quite far enough. I already have some fixes on top of this that I'm testing, but am a little distracted this evening.

[fmoralesc]

@alerque Great to hear! (BTW, sorry I haven't been around more, I've been crunching my thesis.)

@doronbehar I think you are right about the settings, it might even be a good idea to remove pandoc_markdown to simplify things.

[alerque]

@fmoralesc Dropping g:#filetypes#pandoc_markdown would definitely simply things. There are currently not even two but three general ways the plugin could get triggered. This is definitely part of the issue with #342 and general confusion over when this plugin is running vs. what the filetype is.

The ftdetect setting the filetype for *.pandoc etc files makes perfect sense but whas is the actual advantage of force-changing the filetype of things already detected as markdown to pandoc vs. loading the plugin functions anyway and leaving the filetype alone? Right now both ways seems to work for me and I think it would make sense to drop the former (i.e. not have a pandoc_markdown setting at all and just "handle" and load the plugin for markdown file types.

[fmoralesc]

whas is the actual advantage of force-changing the filetype of things

already detected as markdown to pandoc vs. loading the plugin functions anyway and leaving the filetype alone? To load our syntax file instead of the markdown syntax file (I haven't tested how that combination works in ages.) Remember that the original version of vim-pandoc 'bundle' included the syntax file.

…

On Wed, Mar 18, 2020 at 6:43 PM Caleb Maclennan ***@***.***> wrote: @fmoralesc <https://github.com/fmoralesc> Dropping g:#filetypes#pandoc_markdown would definitely simply things. There are currently not even two but three general ways the plugin could get triggered. This is definitely part of the issue with #342 <#342> and general confusion over when this plugin is running vs. what the filetype is. The ftdetect setting the filetype for *.pandoc etc files makes perfect sense but whas is the actual advantage of force-changing the filetype of things already detected as markdown to pandoc vs. loading the plugin functions anyway and leaving the filetype alone? Right now both ways seems to work for me and I think it would make sense to drop the former (i.e. not have a pandoc_markdown setting at all and just "handle" and load the plugin for markdown file types. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#363 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABWCGJGVHBI274RY2ZATPTRIEB5PANCNFSM4LJH6EYQ> .

[doronbehar]

The ftdetect setting the filetype for *.pandoc etc files makes perfect sense but whas is the actual advantage of force-changing the filetype of things already detected as markdown to pandoc vs. loading the plugin functions anyway and leaving the filetype alone?

I agree. Usually, when a plugin introduces a new filetype, it instructs users to add appropriate lines to their filetype.vim, such as:

au BufNewFile,BufRead *.md setf pandoc

Maybe vim-pandoc should drop all pandoc#filetypes# settings and instruct using lines such as this instead?

[fmoralesc]

The reason all of this was setup like this was that ideally the user wouldn't have to configure anything to use the plugin with reasonable defaults. The other reason is that the pandoc functionality is mostly independent of the filetype, due to pandoc's own handling of multiple filetypes; so I designed it to work more or less like an Emacs (gasp!) minor mode...

…

On Wed, Mar 18, 2020 at 7:02 PM Doron Behar ***@***.***> wrote: The ftdetect setting the filetype for *.pandoc etc files makes perfect sense but whas is the actual advantage of force-changing the filetype of things already detected as markdown to pandoc vs. loading the plugin functions anyway and leaving the filetype alone? I agree. Usually, when a plugin introduces a new filetype, it instructs users to add appropriate lines to their filetype.vim, such as: au BufNewFile,BufRead *.md setf pandoc Maybe vim-pandoc should drop all pandoc#filetypes# settings and instruct using lines such as this instead? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#363 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABWCGJ5O3GWQ5JC5CRHIRLRIEEDVANCNFSM4LJH6EYQ> .

[doronbehar]

The reason all of this was setup like this was that ideally the user
wouldn't have to configure anything to use the plugin with reasonable
defaults.

That's understandable, but that's the idea of setfiletype - it changes the filetype (in our case to pandoc) only if filetype wasn't set previously. I think vim-pandoc-syntax should use setfiletype and then instruct users to disable that behavior by using the following in filetype.vim:

" Note that here, because it's the first time a filetype is deceted,
" it doesn't matter if you use `set filetype` or `setfiletype`.
au BufNewFile,BufRead *.md setfiletype markdown

TBH, I've checked with many filetype-introducing plugins and only few of them use setfiletype instead of set filetype, But that's not such a big deal for them as most people don't need this kind of configuration. However, neomutt's filetype plugin is an example:

https://github.com/neomutt/neomutt.vim/blob/98ae21997fe1386961658e056544ad645262d013/ftdetect/neomuttrc.vim

And I found another example in my own set of plugins:

https://github.com/nfnty/vim-nftables/blob/01c7b97eff12fd4b624e6efa2c0468163db61ebc/ftdetect/nftables.vim

You can test that with these plugins installed and along with an appropriate line in your own filetype.vim, you can disable their behavior. The reason this works best is because :scriptnames reports that your own filetype.vim is loaded right after init.vim (at least on Neovim) and hence it sets your selection of filetype before any other external plugin.

[alerque]

As a tide-over until we can get the file type handling worked out to everybody's satisfaction I added a commit to this PR to never bother setting g:pandoc#filetypes#pandoc_markdown at all and instead just always check it with get() (and default to 1). It can still be disabled by setting to 0 as before.