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

New snippets for Verilog and some reorganization #1522

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

mpkopec
Copy link

@mpkopec mpkopec commented Aug 14, 2024

There are a few things changed in this PR.

First of all, I tried to organize the snippets in both Verilog and SystemVerilog sections. I have sorted all the snippets alphabetically to ease anyone editing or searching snippets definitions in the future. It might not be very obvious in the short term (it has separated if from else), but when dealing with large files I find it a lot more consistent to have things laid out in this way.

I have added ${VISUAL} wherever possible in the block snippets leaving only the snippets like if-else untouched, since there is no possible way of knowing which part to put the VISUAL into. Also, I have changed most of the ${0} to ${N}, as it allows UltiSnips users to leave the snippet with the tabstop navigator keystroke.

I also added descriptions to most of the snippets and removed the comments, which were short and unnecessary, especially with the description present. I have left blank lines where the comments were, but this introduces a new line after the snippet, so I am open to suggestions on leaving the blanks there or removing them. This also meant, that I needed to add an empty comment after 2 of the "inline" snippets, so that the engine does not insert the new line. I think, that in most cases the blank line is not only neutral, but nice-to-have.

I have added verilog_systemverilog snippets file simply extending the SystemVerilog ones, since there is a very nice plugin for Verilog and SystemVerilog development adding verilog_systemverilog filetype to the Vim database, which is obviously neither verilog nor systemverilog ft, yielding the snippets unusable. This fixes this issue without creating symlinks, which would not work on Windows.

I have added a few of my own snippets, which I extensively use in my Verilog day-to-day job. I tried to balance the reconfigurability of a particular snippet with its ease of use and minimising the keystrokes for the end user, because that's what snippets are for. This yielded input and output being separate, as the end user would need far more keystrokes when defining multiple inputs/outputs one by one. This also meant that I needed to put some of the snippets into the UltiSnips folder, as UltiSnips could not render nested tabstops from the snippets folder (maybe it's a bug, but I had no time so far to create a reproducible example for the UltiSnips maintainers).

I have not extensively tested the snippets except for the newly added, but since they were not changed I do not expect trouble.

I am open to suggestions and pointing out any mistakes on my side.

@mpkopec
Copy link
Author

mpkopec commented Aug 14, 2024

After using the snippets for a while at work, I found that the additional whitespace at the end is realli irritating, so I removed them.

Copy link
Collaborator

@lpil lpil 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 hard to review as lots have changed. Could you remove the extra comments and undo the order changing please.

The ultisnips snippets don't seem to use any features that the snipmate format do not support, so they need to be removed in favour of snipmate.

@mpkopec
Copy link
Author

mpkopec commented Sep 1, 2024

Hi,
Regarding the order change, from what you wrote, I assume that you don't oppose it in itself, rather it makes it difficult to see changes. If that's the case, maybe instead of adding a bunch of commits reverting and re-introducing it I could provide a diff file/files with such information? If not, I can reorder them back, but this was really hard to navigate.

Indeed, the UltiSnips snippets do not use any additional features, but the nested tabstops in snipmate format seem not to be working with NeoVim + UltiSnips. Would it be possible if you also checked that? I don't know if it's a me problem or the UltiSnips problem. Ofc I agree, that ideally snippets without special UltiSnips features should not be put in UltiSnips, but for me this is what works, and the desired layout does not.

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