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

Verilog: extract module names used for defining instances as reference tags #3473

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

masatake
Copy link
Member

This one is conceptually based on
master...my2817:ctags:master
reported and written by @my2817 at #3469.

The test case is also taken from #3469 submitted by @my2817.

@masatake masatake marked this pull request as draft August 28, 2022 09:34
@masatake masatake requested a review from hirooih August 28, 2022 09:34
@codecov
Copy link

codecov bot commented Aug 28, 2022

Codecov Report

Merging #3473 (b2214c1) into master (934f60e) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3473      +/-   ##
==========================================
+ Coverage   83.33%   83.34%   +0.01%     
==========================================
  Files         219      219              
  Lines       52760    52796      +36     
==========================================
+ Hits        43966    44002      +36     
  Misses       8794     8794              
Impacted Files Coverage Δ
parsers/verilog.c 98.43% <100.00%> (+0.01%) ⬆️
parsers/powershell.c 91.16% <0.00%> (+0.96%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@hirooih hirooih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed on #3469, defInstance is not proper name for the role of the tag.
I think "decl" or "declaration" is better.

Other than this, your fix is clean and readable as usual.
Thank you.

@masatake
Copy link
Member Author

masatake commented Sep 4, 2022

I see. I will update the name of the role. I will keep this draft till I write about decl role n the parser man page.

@hirooih
Copy link
Contributor

hirooih commented Sep 4, 2022

I will update the name of the role.

R_MODULE_DEFINSTANCE was not renamed yet.

I will keep this draft till I write about decl role n the parser man page.

I see.

Copy link
Contributor

@hirooih hirooih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

Please merge this when you think you are ready.

@masatake masatake marked this pull request as ready for review September 4, 2022 16:02
@masatake
Copy link
Member Author

masatake commented Sep 4, 2022

The man page is updated.

@masatake masatake requested a review from hirooih September 4, 2022 16:08
Comment on lines 165 to 181
Supported Roles
~~~~~~~~~~~~~~~

.. warning::

The support for references in Universal Ctags is still
experimental; the names of the roles can be changed in the future.


.. code-block:: console

$ ./ctags --list-roles=SystemVerilog
#KIND(L/N) NAME ENABLED DESCRIPTION
m/module decl on declaring instances

$ ./ctags --list-roles=Verilog
#KIND(L/N) NAME ENABLED DESCRIPTION
m/module decl on declaring instances

The parsers extract modules names used in instance declarations as
reference tags. ``decl`` is the role for the names. See "TAG ENTRIES"
section of :ref:`ctags(1) <ctags(1)>` about reference tags and roles.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Thank you.

Comment on lines 178 to 179
The parsers extract modules names used in instance declarations as
reference tags. ``decl`` is the role for the tags. See "TAG ENTRIES"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

The parser extracts module-names (or names of modules) used in instance declarations as reference tags.

We can say "The parsers extraces ...". If we choose this, we have to change other sentences in this manual.

The man page about SystemVerilog/Verilog parsers for Universal Ctags

This man page describes about the SystemVerilog/Verilog parsers for Universal Ctags.

I prefer singulars.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

…ce tags

This one is conceptually based on
universal-ctags/ctags@master...my2817:ctags:master
reported and written by @my2817 at universal-ctags#3469.

The test case is also taken from universal-ctags#3469 submitted by @my2817.
Copy link
Contributor

@hirooih hirooih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@masatake masatake merged commit 61bf787 into universal-ctags:master Sep 6, 2022
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