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

close #510: update DOC #515

Merged
merged 7 commits into from
Aug 13, 2022
Merged

close #510: update DOC #515

merged 7 commits into from
Aug 13, 2022

Conversation

zjp-CN
Copy link
Contributor

@zjp-CN zjp-CN commented Aug 10, 2022

Since I'm reading the DOC and haven't finished it yet, I will fix more minor mistakes in this PR if I find them.

Plus, I'm translating the documentation here, and it's WIP.
Each example code is tried and the usage is recorded as GIF like this.
I can add them to the DOC if you want them to appear in the DOC.

@L3MON4D3
Copy link
Owner

Since I'm reading the DOC and haven't finished it yet, I will fix more minor mistakes in this PR if I find them.

Oh yes, please 👍👍

Plus, I'm translating the documentation here, and it's WIP.

Wow, That's awesome! Thank you for your effort ❤️
Do you think it would make sense (eg. would any chinese-speakers see it) to add a note pointing to the translation into the Readme?
(Maybe in chinese+english?)

Each example code is tried and the usage is recorded as GIF like this. I can add them to the DOC if you want them to appear in the DOC.

Oh that's a great idea, should help with understanding the snippets. Would adding them as mp4 be much trouble? I feel like being able to pause the playback is really helpful for understanding what's going on, and that's not possible with gifs :/

We're using panvimdoc to convert the markdown to vimdoc, so the gifs/mp4s should probably put inside an ignore-block so they don't show up in :h luasnip

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Aug 10, 2022

I feel like being able to pause the playback is really helpful for understanding what's going on, and that's not possible with gifs :/

Like this?

luasnip.mp4

@L3MON4D3
Copy link
Owner

Yup 👍
Although, after having taken a look at the gifs, the snippets aren't that complicated, maybe pausing isn't really necessary.
I'll leave that to your judgment :D

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Aug 10, 2022

the gifs/mp4s should probably put inside an ignore-block so they don't show up in :h luasnip

Github accepts a plain video link to show the video, so I'm not sure whether the plain link will be displayed in vimdoc.

the snippets aren't that complicated, maybe pausing isn't really necessary

Correct. The main purpose I make GIFs is to know the result of expansion, and all actions in GIFs are simple:

  • type some characters and select
  • stroke tab key to comfirm
  • stroke tab key to jump in and out

I think GIFs is simple and doesn't occupy much visual space, which looks neat.

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Aug 10, 2022

add a note pointing to the translation into the Readme

Of course, I will add it in this PR after I finish the translation. It won't take too long hopefully (2/3 already done). Thank you for reminding me of that.

@L3MON4D3
Copy link
Owner

Github accepts a plain video link to show the video, so I'm not sure whether the plain link will be displayed in vimdoc.

I think it might be shown as-is, you can push a version of DOC.md with a video and take a look at the generated doc/luasnip.txt. In case the links are displayed, it would probably be best to just hide them, the links alone probably don't do much good.
Perhaps a hint at the beginning that the markdown (github-hosted)-version contains videos, which are omitted in vimdoc?

I think GIFs is simple and doesn't occupy much visual space, which looks neat.

Oh I agree. Also, in this case the playback/pause-ui covers a good amount of the content, so that's another downside. Gifs it is 👍

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Aug 11, 2022

docstring is confusing in DOC.

All the examples from there yield the same result without any error, which is hard to understand why the arguments docTrig and docstring is needed and what they mean:

ls.add_snippets("all", {
  s({ trig = "doc_simple(%d)", regTrig = true, }, {
    f(function(args, snip)
      return string.rep("repeatme ", tonumber(snip.captures[1]))
    end, {})
  }),
  s({ trig = "doc_trig(%d)", regTrig = true, docTrig = "2" }, {
    f(function(args, snip)
      return string.rep("repeatme ", tonumber(snip.captures[1]))
    end, {})
  }),
  s({ trig = "docstring(%d)", regTrig = true, docstring = "repeatmerepeatmerepeatme" }, {
    f(function(args, snip)
      return string.rep("repeatme ", tonumber(snip.captures[1]))
    end, {})
  }),
})
2022-08-11_23-41-11.mp4

The only explanation I found lies in the start of DOC:

  • docstring: string, textual representation of the snippet, specified like dscr. Overrides docstrings loaded from json.
  • docTrig: string, for snippets triggered using a lua pattern: define the trigger that is used during docstring-generation.

And no example that must/should use snippet:get_docstring() to generate snippets is shown.

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Aug 11, 2022

One thing I'm suprised at is the table returned in lua file which is loaded by LuaSnip:

return {
	parse("trig", "loaded!!")
}

As defining all of the snippet-constructors (s, c, t, ...) in every file is rather cumbersome, luasnip will bring some globals into scope for executing these files.

With this sentence in mind, I thought something like s("trig". ...) would work:

return {
	parse("trig", { t"loaded!!", i(1, "") })
}

Now, after several trials, I find it really means the lsp-snippets:
ls.parser.parse_snippet and plain parse are actually related, and nothing about snippet-constructors.

@L3MON4D3
Copy link
Owner

docstring is confusing in DOC.

All the examples from there yield the same result without any error, which is hard to understand why the arguments docTrig and docstring is needed and what they mean:

Ahh, that isn't explained at all :/
This problem is very similar to why multiline_vars need to be specified in env_namespace:
Again, if we want to generate a docstring for a snippet (used by eg.
nvim-cmp, and shown in its' doc-window) we need to run the function/dynamicNodes inside the snippet.
Just how variables can't be generated because we don't have a buffer, the
trigger-captures also can't be generated, since we don't have a trigger that
triggered this snippet (we're just calling get_docstring).
(example for a problematic snippet:

	-- has regTrig, (%d) is stored in `snip.captures[1]`
	s({ trig = "doc_simple(%d)", regTrig = true, }, {
	  f(function(args, snip)
	    return string.rep("repeatme ", tonumber(snip.captures[1]))
	  end, {})
	}),

)
Sooo, we chose to pass $CAPTUREi in snip.captures[i], and $TRIGGER in snip.trigger.
But, this can be problematic: in the above case, where captures[1] is, during
regular usage, a number the snippet works just fine, but since
snip.captures[1] returns "$CAPTURE1" (very much not a number) when
get_docstring() is called on the snippet, there would be an error.
To prevent this, one can pass docTrig to still provide a trigger for
get_docstring(), so captures can be populated accurately.
(directly passing docstring also works, but is more annoying if the snippet
changes).

I guess this is, again, a bit in-depth for someone just starting out, so a link
to this explanation might be best.
Or maybe an appendix where the necessity for these fields is motivated
properly, with examples for problems that are solved by them.

@L3MON4D3
Copy link
Owner

One thing I'm suprised at is the table returned in lua file which is loaded by LuaSnip:

return {
	parse("trig", "loaded!!")
}

As defining all of the snippet-constructors (s, c, t, ...) in every file is rather cumbersome, luasnip will bring some globals into scope for executing these files.

With this sentence in mind, I thought something like s("trig". ...) would work:

It should, in files loaded by the lua-loader a call to ls.setup_snip_env() is inserted at the top, before the file is run.
parse is just the name the snippet-parser is inserted as, see here for the defaults.

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Aug 12, 2022

Well, I find this example enlighten me much:

Given the three snippets

local doc1 = s({ trig = "doc(%d)", regTrig = true, }, f(
  function(_args, snip)
    return string.rep("repeatme ", tonumber(snip.captures[1]))
  end, {}
))
local doc2 = s({ trig = "doc(%d)", regTrig = true, docTrig = "doc2" }, f(
  function(_args, snip)
    return string.rep("repeatme ", tonumber(snip.captures[1]))
  end, {}
))
local doc3 = s({ trig = "doc(%d)", regTrig = true, docstring = "repeatmerepeatmerepeatme" }, f(
  function(_args, snip)
    return string.rep("repeatme ", tonumber(snip.captures[1]))
  end, {}
))

add_snippets / directly return

-- in some lua file
ls.add_snippets("all", { doc1, doc2, doc3 })

-- or equivelently via loader, in `all.lua` or `all/*.lua`
return { doc1, doc2, doc3 }

In this case, they generate the same result. The expansion of docN is what repeatme repeats N times.

docstring

local function gen(snip)
  return table.concat(snip:get_docstring())
end
return {
  -- parse("doc_one", gen(doc1)),
  parse("doc_two", gen(doc2)),
  parse("doc_three", gen(doc3)),
  doc1,
}

For doc_one, an error occurs:

Error while evaluating functionNode@1 for snippet 'doc(%d)':
bad argument #2 to '?' (number expected, got nil)

Now here comes the docTrig and docstring!

For doc_two, the expansion is repeatme repeatme . (docTrig = "doc2")
For doc_three, the expansion is repeatmerepeatmerepeatme. (docstring = "repeatmerepeatmerepeatme")

docstring

@L3MON4D3
Copy link
Owner

Nice, yes that's exactly it 👍

@zjp-CN

This comment was marked as spam.

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Aug 12, 2022

It seems all images in md should be manually marked as

<!-- panvimdoc-ignore-start -->

![img](https://...)

<!-- panvimdoc-ignore-end -->

to prevent appearance in vimdoc.

<div class="figure">
<img src="https://user-images.githubusercontent.com/25300418/184359293-7248c2af-81b4-4754-8a85-7a2459f69cfc.gif" title="fig:"/>
<p class="caption">InsertNode</p>
</div>

@L3MON4D3
Copy link
Owner

Woah, that's a lot of gifs, amazing work👍👍👍

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Aug 13, 2022

Preview

All I want to add is done. This PR can be merged after you review.

@L3MON4D3
Copy link
Owner

LGTM, Thank you very much 👍👍

@L3MON4D3 L3MON4D3 merged commit a442dd1 into L3MON4D3:master Aug 13, 2022
@zjp-CN zjp-CN deleted the doc branch August 13, 2022 08:31
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.

2 participants