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

Erlang: display module node in the symbols tree #3837

Merged
merged 1 commit into from Apr 18, 2024
Merged

Conversation

b4n
Copy link
Member

@b4n b4n commented Apr 13, 2024

Properly display the module node itself in the symbols tree so it can be the module content's parent instead of having each content node showing the module name as prefix.

Fixes #2650.

This is a revamp of the old patch I suggested ages ago in #2650 but failed to PR/commit.

@fbrau can you test this and see if all is good for you?

Properly display the module node itself in the symbols tree so it can
be the module content's parent instead of having each content node
showing the module name as prefix.

Fixes geany#2650.
@b4n b4n added the tag-manager Tag manager related patches and bugs label Apr 13, 2024
@b4n b4n added this to the 2.1 milestone Apr 13, 2024
@b4n b4n requested a review from techee April 13, 2024 22:37
@techee
Copy link
Member

techee commented Apr 17, 2024

LGTM.

By the way, I think there are quite a few of these tm_tag_undef_t as I just grabbed the upstream parsers and only mapped those kinds we used previously and left the rest unmapped.

@fbrau
Copy link

fbrau commented Apr 17, 2024

@fbrau can you test this and see if all is good for you?

It works. Me personally dont like having a "modules" root category because in Erlang its redundant and adds an unnecessary level of indentation in the panel... For languages that can declare multiple modules per file, like OCaml for example, that would be reasonable but not for Erlang... Anyways, we have some minor appreciation differences but it works

@b4n b4n self-assigned this Apr 18, 2024
@b4n b4n merged commit 3f60bde into geany:master Apr 18, 2024
7 checks passed
@b4n
Copy link
Member Author

b4n commented Apr 18, 2024

@fbrau OK, thanks for testing. I'm merging this for now because it improves the situation a fair bit, but we can still discuss the details.

For whether the module should be shown or not, it's probably a bit of taste, and a bit of meaning, for example:

  • can anything in an Erlang file declaring a module not be in that module? There still seem to be some tags that are not inside the module (macros, types…): is that correct, or should they also all be in the module scope?
  • can an Erlang file not declare a module? If so, doesn't it make sense to create a tag for the module declaration, and then, doesn't it make sense to set the proper scope on the members?

As mentioned, I'd rather not just "hide" the module scope as a special case, because it's a very specific non-trivial code change that might (or might not?) be partly dictated by your taste (which is fine, but might not be universal).
Also, note that there's not an option in the symbols list not to split symbols into groups (Structs, Macros, etc.) and only the hierarchy as a whole. That's admittedly kind of the opposite of what you're asking for, but depending on your usage and the reasons why you don't like the grouping, you might actually like it (or hate it, turn it off again 🙂 )

@fbrau
Copy link

fbrau commented Apr 19, 2024

I'm merging this for now because it improves the situation a fair bit

Great, I've been using the 1.38 patch without problems, this definitely improves usability a lot.

Here are some points about the language semantics to discuss in the future:

About modules, the specification mentions that -module(Module) declaration should be before
any function definition (aka top of the file) and the module name is to be same as the file name
minus the extension .erl

Can an Erlang file not declare a module?

Erlang .erl files are always modules and must always declare -module(Module).

Function definitions are defined inside .erl files only (the legal way).

Include files (.hrl) which are rendered with the same geany parser, on the other hand,
dont specify -module(MODULENAME). These preprocessed files are not a module though, they are
as hygienic as C header files.

Macros and types can be defined in a module (.erl) or in an include file (.hrl) but they dont actually live
in module scope or header scope or anything like that. They go to global scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag-manager Tag manager related patches and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

items redundantly prefixed with the current filename in symbols panel
3 participants