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 reference to verilog-instance plugin in the README #149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add reference to verilog-instance plugin in the README #149

wants to merge 1 commit into from

Conversation

antoinemadec
Copy link
Contributor

Hi Vitor,

I just added this vim plugin:
https://github.com/antoinemadec/vim-verilog-instance

This creates SystemVerilog port instantiation from port declaration.
It is saving me a lot of time when I code in Verilog.
I have been using it for years now and I just found the courage to add features, re write it and release it to the world.

I was wondering if you could add it to the "Other vim plugins for Verilog/SystemVerilog" section of verilog_systemverilog's README ?
Also, feel free to report bugs or give me advice to make the plug-in clearer and cleaner.

Thanks in advance,
Antoine

@vhda vhda self-assigned this Oct 4, 2017
@vhda
Copy link
Owner

vhda commented Oct 4, 2017

Hi Antoine,

I have a Vim function that does something similar to what you accomplish with your plugin, although mine is much simpler and would not support multiple ports per line. I feel that my plugin should include this functionality, but I have not added my function because it is too simple. I intend to implement such functionality in the future, but I would like to have as following:

  1. Integrate somehow with SuperTab plugin such that when tab is pressed after a module name it would call an "instance completion" function.
  2. This "instance completion" function would then obtain the list of ports of that module from the tags file.

If you feel like implementing this functionality (in Vim Language, not Python please) then I would love to integrate that work into this plugin.

But because the functionality you are sharing in your plugin will eventually conflict with this plugin, I feel that it does not make sense to include such reference in the documentation.

Hope you understand my reasoning.

P.S. I've recently been a father, so please expect some delay in my updates.

@antoinemadec
Copy link
Contributor Author

Hi Vitor,

First, congratulations on your baby :)

Then, regarding verilog-instance, I completely understand your reasoning.
I agree that integrating it directly in verilog_systemverilog would be better.
However here are some remarks/questions:

  1. Using the tags to automatically do it seems a great idea indeed. I think we should have 2 commands: one like verilog-instance, not based on tags and using vim motions (e.g.: gbi(); and the second using the tags to generate it, like "GetVerilogInstanceFromTags". That way people can use both depending on whether they generated the tags or not. What do you think ?

  2. I don't really like the idea of using SuperTab as a dependency for the GetVerilogInstanceFromTags feature. That could indeed interfere with YouCompleteMe, or other similar plugins. This feature should be as independent and orthogonal with other plugins as possible. What do you think ?

  3. I don't know vim-script very well, but what would be the advantages of using vim-script over python ?
    According to the tests I ran, the current python script of verilog-instance is fast enough (I ran it on a module with 300+ ports and tones of ugly comments). Python regex have the avantage of being powerful and easy to read.

In conclusion, I am going to keep verilog-instance alive until it is merged in verilog_systemverilog.
In the mean time, I will be glad to help you integrate those features !
However I am starting a new job in 2 weeks and won't be very responsive in the next few weeks.

Cheers,
Antoine

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