VHDL-LS / rust_hdl

Other
346 stars 64 forks source link

Add source code location to all named entities #302

Closed Schottkyc137 closed 6 months ago

Schottkyc137 commented 6 months ago

Closes #158

Now adds a source span to all named entity declarations. Also adds unnamed statements and declarations (i.e., process statements) to the entity hierarchy (visible, for example, in the outline in VHDL).

Any feedback concerning the format is greatly appreciated. For example, I'm unsure whether if statements and similar should be part of the outline or not.

pidgeon777 commented 6 months ago

For example, I'm unsure whether if statements and similar should be part of the outline or not.

Do you think it would be possible to let the user decide what should be included in the outline and what not? Maybe by using the LSP server settings during init, or the project config file?

Or, all of the basic VHDL if, for etc. tokens could be output, and then filtered out eventually client side.

I think it would be great to also report the when <value> of the case <name> is in the VHDL outline (useful to quickly jump to FSM states, for example).

Schottkyc137 commented 6 months ago

For example, I'm unsure whether if statements and similar should be part of the outline or not.

Do you think it would be possible to let the user decide what should be included in the outline and what not? Maybe by using the LSP server settings during init, or the project config file?

Or, all of the basic VHDL if, for etc. tokens could be output, and then filtered out eventually client side.

I'm not against a generic configuration. If you could provide a 'user perspective', i.e., how a user would like to configure the various options, I might include it if it isn't too much effort. That being said, such a feature isn't high on my priority list. Therefore I won't give any guarantees that this will actually be implemented. Probably this would belong to the initial server configuration as this is quite specific to the language server.

I think it would be great to also report the when <value> of the case <name> is in the VHDL outline (useful to quickly jump to FSM states, for example).

I see the benefit in such a feature, but I won't implement it now. At the moment, the outline only contains named object, i.e., entities, architectures, e.t.c. This also includes objects that could have a name in form of a label, such as if or case statements. when clauses do not fall into this category and would thus potentially require a more substantial refactor of the current design.

Schottkyc137 commented 6 months ago

@kraigher I'm sort of at a crossroad here. The source location of an AnyEnt is currently implemented using a TokenSpan object while the declaration position is implemented using a SrcPos object. I would like to make this more consistent so that either token information or source code location is given (i.e., either exchanging the declaration position with a single TokenId or exchanging the whole span with a single SrcPos location). Having the SrcPos on an entity has the advantage of being easier, since no context is required to get the actual location, but may be less flexible since finding the original token span is more cumbersome. Having token information could be advantageous however, for example, for extracting documentation during hover. Do you have a preference how this should be implemented?

kraigher commented 6 months ago

The reason to move in direction of TokenSpans was to enable better access to comments and lossless formatting. So it makes sense to keep going in this direction.

Using the context to get tokens is not so bad. Having a context on the side also means less information has to be stored in the data structure themselves which is more space efficient.