Original Redmine Comment
Author Name: David Shleifman
Original Date: 2016-03-14T20:33:21Z

Comment Rollover

I am replying to the comments posted after the issue1025 has been closed.

A. Support rand, randc, and pure

In reply to Wilson Snyder comment (refer to issue1025-12, note #12):

Also the patch as provided should probably also support the rand, randc, and pure qualifiers so that the parsing satisfies the language.

We already improved the +imenu+ (see issue1025-11, note #11). IMHO, it does already a decent job. The thing is that there are two camps of users:

• these who utilize +imenu+ are already enjoying the fruits of the aforesaid improvement.
• these who utilize the speedbar aren't enjoying the fruits, because Verilog/SystemVerilog files are not displayed in the speedbar.

That is why, I propose to elevate priority of the issue1046 over the additional enhanancments of the +imenu+.

B. Generic functions

In reply to Wilson Snyder comment (refer to issue1025-12, note #12):

Furthermore, you cannot generally define functions in verilog-mode that are not part of the verilog- scope, e.g. you can't create a alist-keys (and I suspect anyways there's a more general function so this isn't needed.)

I completely support the development practice: refrain from defining functions in verilog-mode that are not part of the verilog-scope.

B-1. My excuse

After googling it for several hours, I was surprised to find that elisp doesn't have a generic function to retrieve keys from these elements of an associative list that match a certain criterion. Given that such function does not exist, I went ahead and defined it on my own.

B-2. Alternatives

a) Let me know if anybody finds a better alternative.

b) It appears, that @elisp@ does not have namespaces (refer to "EmacsWiki":https://www.emacswiki.org/emacs/Namespaces). I can rename @alist-keys@ to @verilog--alist-keys@ to avoid symbol collision. Let me know if this an acceptable approach.

Patch

I am submitting the @verilog-mode-2016-02-24-3bb6198-vpo.patch@ (attached).

README

1. Major Problem: imenu categories (defined for the verilog-mode) do not appear in the speedbar. Solution: fixed by implemeting the idea mentioned in the issue1025-2 (post Auto-comment and indentation improvements #2).

2. Problem: the following elements do not appear in the imenu: Solution: fixed.

interface class
virtual protected task 
virtual protected function
function automatic

3. Minor Problem: imenu supports the following elements:

static class
local class
protected class

in spite the fact that they do not conform to the SystemVerilog LRM. Solution: retired support for these elements.

HOW-TO apply the patch

It is assumed that @verilog-mode.el@ lives in @~/elisp@.

cd ~
1. download verilog-mode-2016-02-24-3bb6198-vpo.patch to this directory
cp -r elisp elisp.new
cd elisp.new
patch -c -p 1 -i ../verilog-mode-2016-02-24-3bb6198-vpo.patch
1. expected output
patching file verilog-mode.el

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-03-14T22:38:26Z

1. Please add randc/rand to your regexps, I see no downside.

2. You could use verilog-alist-keys, but I think having a single use of verilog-alist-keys, and verilog-auto-mode-p isn't that helpful, just flatten the function into speedbar-integrate-verilog-mode. Also that function needs to be verilog-.... prefixed. I think you should see how e.g. vhdl-speedbar-initalize does it, including how it is called and the related declare-function's right above.

Original Redmine Comment
Author Name: David Shleifman
Original Date: 2016-03-16T16:48:15Z

A. Support rand, randc, and pure

In reply to Wilson Snyder comment (refer to issue1046-2, note #2):

1. Please add randc/rand to your regexps, I see no downside.
   In respect to the Variables category of the +imenu+, I see no great need to enhance it. There are roughly two camps of users:

1. these who edit Verilog modules, RTL in particular. Their primary need is to see the list of nets, and selected types of variables, such as @logic@, and @reg@. I inspected +imenu+ patterns, and found that these nets and variables are already included in the +imenu+.
2. these who edit SystemVerilog classes. Their primary need is to see the list of methods: tasks and functions. As for the class members (regardless whether a class member has @rand@ or @RandC@ qualifiers or not), they usually appear at the top of the class definition, as opposed to being scattered all over the places in the source file. In that sense, adding them to the Variables category does not seem to be of a great value to users, does it? Kindly, share your opinion on this aspect.

Note that I deviated from the above approach in respect to the pure methods. As a justification, I considered the case of a mix of pure and non-pure methods. In this case, +imenu+ produces an incomplete list of methods. The incomplete list of methods may mislead the user. To avoid a potential harm, I added pure methods to the Tasks and Functions categories.

B. Generic functions

In reply to Wilson Snyder comment (refer to issue1046-2, note #2):

2. You could use verilog-alist-keys, but I think having a single use of verilog-alist-keys, and verilog-auto-mode-p isn't that helpful, just flatten the function into speedbar-integrate-verilog-mode. Also that function needs to be verilog-.... prefixed. I think you should see how e.g. vhdl-speedbar-initalize does it, including how it is called and the related declare-function's right above.
   Thank you for the excellent feedback and suggestion. I flattened @alist-keys@ into the speedbar initialization function, which closely resembles @vhdl-speedbar-initalize@.

Patch

I am submitting the @verilog-mode-2016-02-24-3bb6198-vpo.2.patch@ (attached).

README

1. Major Problem: imenu categories (defined for the verilog-mode) do not appear in the speedbar. Solution: fixed by implemeting the idea mentioned in the issue1025-2 (post Auto-comment and indentation improvements #2).

2. Problem: the following elements do not appear in the imenu: Solution: fixed.

interface class
virtual protected task 
virtual protected function
function automatic
pure virtual task
pure virtual function

3. Minor Problem: imenu supports the following elements:

static class
local class
protected class

in spite the fact that they do not conform to the SystemVerilog LRM. Solution: retired support for these elements.

HOW-TO apply the patch

It is assumed that @verilog-mode.el@ lives in @~/elisp@.

cd ~
1. download verilog-mode-2016-02-24-3bb6198-vpo.2.patch to this directory
cp -r elisp elisp.new
cd elisp.new
patch -c -p 1 -i ../verilog-mode-2016-02-24-3bb6198-vpo.patch
1. expected output
patching file verilog-mode.el

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-03-17T00:30:31Z

Great, thanks for adapting. I made one minor change to use eval-after-load as it saves a few lines.

Pushed to git and download pages in 2016-03-16-4e283c6-vpo