Imenu for cobol-mode

Imenu for cobol-mode

[Joakim Jalap]

[-- Attachment #1: Type: text/plain, Size: 1258 bytes --]

Hello Emacs Devs and Edward!

Here is my attempt at adding imenu functionality to cobol-mode.

I have tried it on a bunch of programs from the gnu cobol contrib repo, and IMHO
it seems to work pretty well :)

I've noticed one false positive; in the case of a procedure division header
which looks like this:

procedure division chaining input-file-name output-file-name
    change.

"change" will wrongly be recognized as a paragraph (is that what it is called?),
ie a function. I guess that could be worked around by making the regexp I use to
search for the procedure division match everything including the ending "."...
Well, that's for another day. I've also only tried this on complete programs, so
I'm not exactly sure how it reacts to a half finished buffer, but I think it
should be allright.

I would be glad if somebody who is a cobol programmer could give it a try and
provide some feedback (I know only of one cobol programmer who uses Emacs so I
cc:d him :))

To try it, eval the attached file (you must load cobol-mode first), visit a
buffer in cobol mode and eval:
(progn
 (setq-local imenu-create-index-function 'cobol--imenu-create-index)
 (imenu-add-to-menubar "Imenu"))

then browse away.

Looking forward to your feedback :)

-- Joakim


[-- Attachment #2: cobol-mode-imenu --]
[-- Type: application/emacs-lisp, Size: 6082 bytes --]

Re: Imenu for cobol-mode

[Edward Hart]

[-- Attachment #1: Type: text/plain, Size: 2713 bytes --]

Hi Joakim,

The Imenu functionality is excellent and I can't wait to get it committed.

I've only found one bug in the patch which involves function definitions
and program definitions in the same file. If I create an index for such a
file, the index entries for the first program/function are not listed under
the program/function's name but just "File section", "Working-storage
section", etc. The entries for the following programs/functions are listed
under their names, as expected.

I have a few suggestions for the code itself:

   - By convention, COBOL keywords are written in uppercase and I think the
   regexps should be changed to reflect that.
   - The code assumes all programs have IDENTIFICATION DIVISION headers,
   however the header is optional. Match against PROGRAM/FUNCTION-ID instead
   using cobol--function-id-name-re instead (which is equivalent to
   cobol--imenu-program-name).
   - The "^[ \t]*\\([[:digit:]]\\{1,2\\}\\|[fsr]d\\)[ \t]+\\(\\w+\\)"
   regexp can be replaced with cobol--generic-declaration-re.

Two very useful features I'd like to suggest would be peeking at
(displaying the line a data item is defined on in a temporary buffer would
be good enough) and jumping to data definitions. Adding key bindings for
them would be an added bonus.

Regards,
Edward

On 3 March 2017 at 18:24, Joakim Jalap <joakim.jalap@fastmail.com> wrote:

> Hello Emacs Devs and Edward!
>
> Here is my attempt at adding imenu functionality to cobol-mode.
>
> I have tried it on a bunch of programs from the gnu cobol contrib repo,
> and IMHO
> it seems to work pretty well :)
>
> I've noticed one false positive; in the case of a procedure division header
> which looks like this:
>
> procedure division chaining input-file-name output-file-name
>     change.
>
> "change" will wrongly be recognized as a paragraph (is that what it is
> called?),
> ie a function. I guess that could be worked around by making the regexp I
> use to
> search for the procedure division match everything including the ending
> "."...
> Well, that's for another day. I've also only tried this on complete
> programs, so
> I'm not exactly sure how it reacts to a half finished buffer, but I think
> it
> should be allright.
>
> I would be glad if somebody who is a cobol programmer could give it a try
> and
> provide some feedback (I know only of one cobol programmer who uses Emacs
> so I
> cc:d him :))
>
> To try it, eval the attached file (you must load cobol-mode first), visit a
> buffer in cobol mode and eval:
> (progn
>  (setq-local imenu-create-index-function 'cobol--imenu-create-index)
>  (imenu-add-to-menubar "Imenu"))
>
> then browse away.
>
> Looking forward to your feedback :)
>
> -- Joakim
>
>

[-- Attachment #2: Type: text/html, Size: 3392 bytes --]

Re: Imenu for cobol-mode

[Joakim Jalap]

Edward Hart <edward.dan.hart@gmail.com> writes:

> Hi Joakim,

Hi!

> The Imenu functionality is excellent and I can't wait to get it committed.

Great :)

> I've only found one bug in the patch which involves function
> definitions and program definitions in the same file. If I create an
> index for such a file, the index entries for the first
> program/function are not listed under the program/function's name but
> just "File section", "Working-storage section", etc. The entries for
> the following programs/functions are listed under their names, as
> expected.

That was actually intentional, or well, semi intentional anyway ;) I was
debating (with myself) what would look best, I had some idea that the
first program/function definition in the file would be the "main" one
and that that should get the "main" menu items. But I will change it
so that all subprograms are under their respective names. Maybe also put
the subprogram submenu first?

> I have a few suggestions for the code itself:
>
> * By convention, COBOL keywords are written in uppercase and I think
> the regexps should be changed to reflect that.

Will do!

> * The code assumes all programs have IDENTIFICATION DIVISION headers, however the header is optional. Match against PROGRAM/FUNCTION-ID instead using cobol--function-id-name-re instead (which is
>  equivalent to cobol--imenu-program-name).

Huh, I had no idea that header was optional :) I will change it to
cobol--function-id-name-re (I actually meant to use the already defined
regexps all along, but I got slightly lost in the maze of all regexps :))

> * The "^[ \t]*\\([[:digit:]]\\{1,2\\}\\|[fsr]d\\)[ \t]+\\(\\w+\\)" regexp can be replaced with cobol--generic-declaration-re.

Likewise.

> Two very useful features I'd like to suggest would be peeking at
> (displaying the line a data item is defined on in a temporary buffer
> would be good enough) and jumping to data definitions. Adding key
> bindings for them would be an added bonus.

I don't really understand what you mean here. Isn't jumping to
definitions what Imenu does? :) Or do you mean something more xref-like?

Does xref have a generic imenu backend? Should it?

Actually I think that peeking should be a feature of xref (xref-peek?).
It sounds a bit like xref-show-location-at-point, but maybe not really?

Anyway, I think that these things are maybe better left to a
xref-backend for cobol-mode (which I would love to write :)). In general
I'm quite excited at the prospect of having uniform keybindings and
interfaces across modes for things like go to definition (and peek at
definition).

Come to think of it, ctags already support COBOL, so you should be able
to get xref functionality via that, I think.

> Regards,
> Edward

Thanks for the feedback! I will try to have a new version ready before
the end of the week.

-- Joakim

Re: Imenu for cobol-mode

[Edward Hart]

[-- Attachment #1: Type: text/plain, Size: 3614 bytes --]

>
> I don't really understand what you mean here. Isn't jumping to definitions
> what Imenu does? :)


Yes, you're right. Sorry, I should check my emails more carefully before I
send them.

Or do you mean something more xref-like? ...  Come to think of it, ctags
> already support COBOL, so you should be able

to get xref functionality via that, I think.


I don't know; you've actually introduced me to xref. I have heard of ctags,
but I've yet to figure out how to use it.

I will try to have a new version ready before the end of the week.


Exciting!

On 6 March 2017 at 08:58, Joakim Jalap <joakim.jalap@fastmail.com> wrote:

> Edward Hart <edward.dan.hart@gmail.com> writes:
>
> > Hi Joakim,
>
> Hi!
>
> > The Imenu functionality is excellent and I can't wait to get it
> committed.
>
> Great :)
>
> > I've only found one bug in the patch which involves function
> > definitions and program definitions in the same file. If I create an
> > index for such a file, the index entries for the first
> > program/function are not listed under the program/function's name but
> > just "File section", "Working-storage section", etc. The entries for
> > the following programs/functions are listed under their names, as
> > expected.
>
> That was actually intentional, or well, semi intentional anyway ;) I was
> debating (with myself) what would look best, I had some idea that the
> first program/function definition in the file would be the "main" one
> and that that should get the "main" menu items. But I will change it
> so that all subprograms are under their respective names. Maybe also put
> the subprogram submenu first?
>
> > I have a few suggestions for the code itself:
> >
> > * By convention, COBOL keywords are written in uppercase and I think
> > the regexps should be changed to reflect that.
>
> Will do!
>
> > * The code assumes all programs have IDENTIFICATION DIVISION headers,
> however the header is optional. Match against PROGRAM/FUNCTION-ID instead
> using cobol--function-id-name-re instead (which is
> >  equivalent to cobol--imenu-program-name).
>
> Huh, I had no idea that header was optional :) I will change it to
> cobol--function-id-name-re (I actually meant to use the already defined
> regexps all along, but I got slightly lost in the maze of all regexps :))
>
> > * The "^[ \t]*\\([[:digit:]]\\{1,2\\}\\|[fsr]d\\)[ \t]+\\(\\w+\\)"
> regexp can be replaced with cobol--generic-declaration-re.
>
> Likewise.
>
> > Two very useful features I'd like to suggest would be peeking at
> > (displaying the line a data item is defined on in a temporary buffer
> > would be good enough) and jumping to data definitions. Adding key
> > bindings for them would be an added bonus.
>
> I don't really understand what you mean here. Isn't jumping to
> definitions what Imenu does? :) Or do you mean something more xref-like?
>
> Does xref have a generic imenu backend? Should it?
>
> Actually I think that peeking should be a feature of xref (xref-peek?).
> It sounds a bit like xref-show-location-at-point, but maybe not really?
>
> Anyway, I think that these things are maybe better left to a
> xref-backend for cobol-mode (which I would love to write :)). In general
> I'm quite excited at the prospect of having uniform keybindings and
> interfaces across modes for things like go to definition (and peek at
> definition).
>
> Come to think of it, ctags already support COBOL, so you should be able
> to get xref functionality via that, I think.
>
> > Regards,
> > Edward
>
> Thanks for the feedback! I will try to have a new version ready before
> the end of the week.
>
> -- Joakim
>

[-- Attachment #2: Type: text/html, Size: 5542 bytes --]

Re: Imenu for cobol-mode

[Joakim Jalap]

[-- Attachment #1: Type: text/plain, Size: 2099 bytes --]

Edward Hart <edward.dan.hart@gmail.com> writes:

> Hi Joakim,
>
> I've only found one bug in the patch which involves function
> definitions and program definitions in the same file. If I create an
> index for such a file, the index entries for the first
> program/function are not listed under the program/function's name but
> just "File section", "Working-storage section", etc. The entries for
> the following programs/functions are listed under their names, as
> expected.

I have now changed it so that if there is only one subprogram the menu
items are the full "WORKING-STORAGE SECTION", "FILE SECTION" etc, but if
there is more than one every subprogram's items are listed under the
respective subprogram name.

> I have a few suggestions for the code itself:
>
> * By convention, COBOL keywords are written in uppercase and I think
> the regexps should be changed to reflect that.

Done.

> * The code assumes all programs have IDENTIFICATION DIVISION headers,
> however the header is optional. Match against PROGRAM/FUNCTION-ID
> instead using cobol--function-id-name-re instead (which is equivalent
> to cobol--imenu-program-name).

Done.

> * The "^[ \t]*\\([[:digit:]]\\{1,2\\}\\|[fsr]d\\)[ \t]+\\(\\w+\\)"
> regexp can be replaced with cobol--generic-declaration-re.

And done :)

I discovered another thing I had overlooked: There need not be
paragraphs (or sections or whatever they're called) in the procedure
division. Some programs just have the code directly there. So now the
procedure division itself gets an entry under "PROCEDURE DIVISION" or
"$subprogram_name PD" with the name of the subprogram/function. WDYT?

> Two very useful features I'd like to suggest would be peeking at
> (displaying the line a data item is defined on in a temporary buffer
> would be good enough) and jumping to data definitions. Adding key
> bindings for them would be an added bonus.

I took the liberty of hacking up something :) That's the last three
functions in the file. It's very ugly and I did it mostly to see if it
could work, and well, it seems to work a little at least.

-- Jocke


[-- Attachment #2: version 2 --]
[-- Type: application/emacs-lisp, Size: 8146 bytes --]

Re: Imenu for cobol-mode

[Joakim Jalap]

Joakim Jalap <joakim.jalap@fastmail.com> writes:

I will humbly ping this while I still have it relatively fresh in my
mind :)
Has anybody had a chance to look at this?

> Edward Hart <edward.dan.hart@gmail.com> writes:
>
>> Hi Joakim,
>>
>> I've only found one bug in the patch which involves function
>> definitions and program definitions in the same file. If I create an
>> index for such a file, the index entries for the first
>> program/function are not listed under the program/function's name but
>> just "File section", "Working-storage section", etc. The entries for
>> the following programs/functions are listed under their names, as
>> expected.
>
> I have now changed it so that if there is only one subprogram the menu
> items are the full "WORKING-STORAGE SECTION", "FILE SECTION" etc, but if
> there is more than one every subprogram's items are listed under the
> respective subprogram name.
>
>> I have a few suggestions for the code itself:
>>
>> * By convention, COBOL keywords are written in uppercase and I think
>> the regexps should be changed to reflect that.
>
> Done.
>
>> * The code assumes all programs have IDENTIFICATION DIVISION headers,
>> however the header is optional. Match against PROGRAM/FUNCTION-ID
>> instead using cobol--function-id-name-re instead (which is equivalent
>> to cobol--imenu-program-name).
>
> Done.
>
>> * The "^[ \t]*\\([[:digit:]]\\{1,2\\}\\|[fsr]d\\)[ \t]+\\(\\w+\\)"
>> regexp can be replaced with cobol--generic-declaration-re.
>
> And done :)
>
> I discovered another thing I had overlooked: There need not be
> paragraphs (or sections or whatever they're called) in the procedure
> division. Some programs just have the code directly there. So now the
> procedure division itself gets an entry under "PROCEDURE DIVISION" or
> "$subprogram_name PD" with the name of the subprogram/function. WDYT?
>
>> Two very useful features I'd like to suggest would be peeking at
>> (displaying the line a data item is defined on in a temporary buffer
>> would be good enough) and jumping to data definitions. Adding key
>> bindings for them would be an added bonus.
>
> I took the liberty of hacking up something :) That's the last three
> functions in the file. It's very ugly and I did it mostly to see if it
> could work, and well, it seems to work a little at least.
>
> -- Jocke

Re: Imenu for cobol-mode

[Edward Hart]

[-- Attachment #1: Type: text/plain, Size: 3359 bytes --]

HI Joakim,

Sorry for not getting back to you sooner (my problem sheets have become
quite difficult this term).

The peek and goto features work very well. I also appreciate the extra code
comments.

So now the procedure division itself gets an entry under "PROCEDURE
> DIVISION" or "$subprogram_name PD" with the name of the
> subprogram/function. WDYT?


I think "PD header" would be a good name.

I noticed you added (require 'seq) to the file. What do you use the library
for? It would be nice to avoid having dependencies.

I've found one extra bug: the functions don't index index-names. Their
declaration is of the form "INDEXED \\(BY\\)? (\w+)". It won't be possible
to merge it with the generic declaration regexp, since the INDEXED clause
often appears at the ends of lines, within another item's declaration.

Edward


On 15 March 2017 at 19:35, Joakim Jalap <joakim.jalap@fastmail.com> wrote:

> Joakim Jalap <joakim.jalap@fastmail.com> writes:
>
> I will humbly ping this while I still have it relatively fresh in my
> mind :)
> Has anybody had a chance to look at this?
>
> > Edward Hart <edward.dan.hart@gmail.com> writes:
> >
> >> Hi Joakim,
> >>
> >> I've only found one bug in the patch which involves function
> >> definitions and program definitions in the same file. If I create an
> >> index for such a file, the index entries for the first
> >> program/function are not listed under the program/function's name but
> >> just "File section", "Working-storage section", etc. The entries for
> >> the following programs/functions are listed under their names, as
> >> expected.
> >
> > I have now changed it so that if there is only one subprogram the menu
> > items are the full "WORKING-STORAGE SECTION", "FILE SECTION" etc, but if
> > there is more than one every subprogram's items are listed under the
> > respective subprogram name.
> >
> >> I have a few suggestions for the code itself:
> >>
> >> * By convention, COBOL keywords are written in uppercase and I think
> >> the regexps should be changed to reflect that.
> >
> > Done.
> >
> >> * The code assumes all programs have IDENTIFICATION DIVISION headers,
> >> however the header is optional. Match against PROGRAM/FUNCTION-ID
> >> instead using cobol--function-id-name-re instead (which is equivalent
> >> to cobol--imenu-program-name).
> >
> > Done.
> >
> >> * The "^[ \t]*\\([[:digit:]]\\{1,2\\}\\|[fsr]d\\)[ \t]+\\(\\w+\\)"
> >> regexp can be replaced with cobol--generic-declaration-re.
> >
> > And done :)
> >
> > I discovered another thing I had overlooked: There need not be
> > paragraphs (or sections or whatever they're called) in the procedure
> > division. Some programs just have the code directly there. So now the
> > procedure division itself gets an entry under "PROCEDURE DIVISION" or
> > "$subprogram_name PD" with the name of the subprogram/function. WDYT?
> >
> >> Two very useful features I'd like to suggest would be peeking at
> >> (displaying the line a data item is defined on in a temporary buffer
> >> would be good enough) and jumping to data definitions. Adding key
> >> bindings for them would be an added bonus.
> >
> > I took the liberty of hacking up something :) That's the last three
> > functions in the file. It's very ugly and I did it mostly to see if it
> > could work, and well, it seems to work a little at least.
> >
> > -- Jocke
>

[-- Attachment #2: Type: text/html, Size: 4853 bytes --]

Re: Imenu for cobol-mode

[Joakim Jalap]

Edward Hart <edward.dan.hart@gmail.com> writes:

> HI Joakim,

Hi again :)

> Sorry for not getting back to you sooner (my problem sheets have
> become quite difficult this term).

No problem at all :)

> The peek and goto features work very well. I also appreciate the extra
> code comments.

Well the peek and goto functions are somewhat dirty hacks, the real
solution there would be to use xref. But would you like to keep them in anyway?

>>  So now the procedure division itself gets an entry under "PROCEDURE DIVISION" or "$subprogram_name PD" with the name of the subprogram/function. WDYT?
>
> I think "PD header" would be a good name.

OK. "PD header" or "PD HEADER"?

> I noticed you added (require 'seq) to the file. What do you use the
> library for? It would be nice to avoid having dependencies.

Just for seq-find in cobol--find-definition. I just got confused by the
many different ways to find something in a list, and seq-find seemed
like the simplest option. I could use something else.

> I've found one extra bug: the functions don't index index-names. Their
> declaration is of the form "INDEXED \\(BY\\)? (\w+)". It won't be
> possible to merge it with the generic declaration regexp, since the
> INDEXED clause often appears at the ends of lines, within another
> item's declaration.

Right, I will get to work on this :)

-- Jocke

Re: Imenu for cobol-mode

[Noam Postavsky]

On Thu, Mar 16, 2017 at 3:56 AM, Joakim Jalap <joakim.jalap@fastmail.com> wrote:
>
>> I noticed you added (require 'seq) to the file. What do you use the
>> library for? It would be nice to avoid having dependencies.
>
> Just for seq-find in cobol--find-definition. I just got confused by the
> many different ways to find something in a list, and seq-find seemed
> like the simplest option. I could use something else.

Since Emacs 25, seq comes with Emacs.

Re: Imenu for cobol-mode

[Joakim Jalap]

[-- Attachment #1: Type: text/plain, Size: 763 bytes --]

Edward Hart <edward.dan.hart@gmail.com> writes:

> HI Joakim,
>
> I think "PD header" would be a good name.

I made it "PD HEADER", but that can be changed.

> I noticed you added (require 'seq) to the file. What do you use the
> library for? It would be nice to avoid having dependencies.

Since Noam said it is included, I left it in for now.
>
> I've found one extra bug: the functions don't index index-names. Their
> declaration is of the form "INDEXED \\(BY\\)? (\w+)". It won't be
> possible to merge it with the generic declaration regexp, since the
> INDEXED clause often appears at the ends of lines, within another
> item's declaration.

Ok, I added this now. I tried on a couple of programs from
gnu-cobol-contrib, and it seems to work :)

-- Joakim


[-- Attachment #2: version 3 --]
[-- Type: application/emacs-lisp, Size: 9304 bytes --]

Re: Imenu for cobol-mode

[Joakim Jalap]

I will once again softly ping this issue!

Joakim Jalap <joakim.jalap@fastmail.com> writes:

> Edward Hart <edward.dan.hart@gmail.com> writes:
>
>> HI Joakim,
>>
>> I think "PD header" would be a good name.
>
> I made it "PD HEADER", but that can be changed.
>
>> I noticed you added (require 'seq) to the file. What do you use the
>> library for? It would be nice to avoid having dependencies.
>
> Since Noam said it is included, I left it in for now.
>>
>> I've found one extra bug: the functions don't index index-names. Their
>> declaration is of the form "INDEXED \\(BY\\)? (\w+)". It won't be
>> possible to merge it with the generic declaration regexp, since the
>> INDEXED clause often appears at the ends of lines, within another
>> item's declaration.
>
> Ok, I added this now. I tried on a couple of programs from
> gnu-cobol-contrib, and it seems to work :)
>
> -- Joakim