-
Notifications
You must be signed in to change notification settings - Fork 2k
Update Rust snippets #528
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
Update Rust snippets #528
Conversation
Add descriptions to Snipmate snippets, update some snippets for changed/deprecated Rust features, add some new bits.
snippet let "let variable declaration" | ||
let ${1:name}${2:: ${3:type}} = ${4}; | ||
snippet letm "let mut variable declaration" | ||
let mut ${1:name}${2:: ${3:type}} = ${4}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are so commonly used that I think it's better to have two separate triggers than one with either two definitions or a tabstop for mut
that needs to be deleted when unwanted.
Note that I only assume the nested tabstops work based on the Snipmate documentation, as noted in the main PR comment UltiSnips doesn't seem to support them in the Snipmate compatibility layer, so these are duplicated in the UltiSnips snippets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not supporting them was intentional, because having them broke some snipMate snippets. that they now support them suckz. Do they now support it for all snippets or only for version 2?
I've pushed small add-on commits here, I'll squash them when ready to merge. |
github ate the reply I wrote here :(. The tl;dr: the merge request lgtm, it suckz that snipMate now added a version 2 file format - does it now always support recursive tabstops or only for version 2? And does it make sense to still have this snippet repo given that every single snippet in here is only partially supported by all snippet engines? |
SnipMate only supports it for version 1, which has to be specifically requested by the user either in the snippets file or via a global Vim variable. In the absence of either---that is by default---version 0 is used which should function the same as before nested stop support was added. I felt adding the separate version was better than forcing a backwards incompatible change, in the interest of SnipMate users. In any case, this should allow this repo to continue as is until contributors decide to move to the new version, if ever. |
Are we happy to merge? |
From my point of view yes, then snippets are of high quality. Sorry to have derailed this with engine incompatibility issues. |
Marvellous. Thank you @ches, everyone. |
Thanks everyone, I was traveling or I might have responded sooner to squash my messy extra commits before merge. Based on @ajzafar's answer above I believe the Snipmate file should have a |
I don't think we care that much about commit history here. :) Thanks ches! |
Yes, SnipMate snippet version 1 is required for nested tab stops. For what it's worth, version 0 and version 1 snippets can be in the same file. I realize this wasn't clear in the documentation, and I've rectified that. |
I suggest keeping version 1 snippets out of vim-snippets (see here #536 (comment)). |
Does this mean we should create a new directory for version 1 snippets? |
Should we? Is the idea to have a common basic snippet directory that can be parsed by all engines and a specific one that can only be read by one engine? It makes sense on paper, but it also limits the usefulness of snippets - already now I feel that the UltiSnips snippets are not fully realized because people hold back to keep them snipMate compatible. Instead of going that route, we can just split the repo again - having one for common basic snippets (version 0) and specific repos for each engine. Or just give up on the unification and not have a common repo at all any more. |
Several things here:
<C-Tab>
listing, the large set of (usually Snipmate) snippets without descriptions drives me crazy. Particularly in cases like thelet
trigger before these changes, where there were two snippets with the same trigger (forlet
andlet mut
), with no hint about which one I was going to get from a menu selection. 😩typ
trigger works around an issue in Expanding a second snippet while the first tabstop of a snippet is selected that spans $0 crashes. SirVer/ultisnips#450. The failure should probably be addressed in UltiSnips eventually, but for now this avoids an out-of-the-box breakage for any Rust user on current UltiSnips and vim-snippets. Plus I'm pretty unlikely to remembertyp
as the trigger anyway, personally, and it only saved one whopping keystroke.For Snipmate
General questions for Snipmate experts since I switched to UltiSnips years ago: does anything here need a
version 1
parser requirement added to this file? Descriptions, nested tabstops on the updatedlet
snippets? A few existing snippets had UltiSnips-style options after the description (snippet der "#[deriving(..)]" b
)—I don't think Snipmate supports these, but they are probably just ignored or considered part of description by the parser. I copied them onto a few related snippets but will remove them if I should. It'd be great if the Snipmate vimdoc page had a section with clear bullet points of what is or isn't supported by the different parser versions.Some newer Snipmate features like nested tabstops don't seem to be supported by the UltiSnips compatibility mode. Thus if I define snippets in a Snipmate file using these features, they won't work in UltiSnips and I need to duplicate them in the UltiSnips file.
It's a troublesome thing about a common repository for Snipmate and UltiSnips: as an UltiSnips user I would like to define Snipmate-compatible snippets in only one place and have them available to all users, but I'm unlikely to actually go to the trouble of testing with Snipmate... (Sorry).
cc @SirVer