rust_hdl icon indicating copy to clipboard operation
rust_hdl copied to clipboard

Add support for directories or file patterns in vhdl_ls.toml

Open Bochlin opened this issue 5 years ago • 6 comments

Being able to configure the language server to recursively include a directory would simplify project setup. A common practice is to have all RTL sources in a single directory and it should be possible to point to this directory in vhdl_ls.toml. Implementing support for directory references would remove the need to update vhdl_ls.toml when new files are added to a project.

To account for some variability, it could also be possible to configure include and exclude patterns for the files within the directory.

Suggestion to expand vhdl_ls.toml to:

  1. Support glob-style patterns in library.files (glob::glob?)
  2. Allow entry in files array to be a directory (checked during config parse) in which case the directory is searched recursively for .vhd files (**/*.vhd)
  3. Add excludes field to library to filter out unwanted files. Supports the same pattern styles as library.files.

Example:

[libraries]
lib1.files = [
    'src',
    'tests/*/tb_*.vhd',
]
lib1.exclude = [
    'test/*/*_mock.vhd',
]

Bochlin avatar Nov 17 '19 14:11 Bochlin

Some design considerations are:

1

Should an error message be given if the pattern does not match any file?

  • If no to the above, what if the pattern is just a plain file name that is missing?

I think an error in case of a missing plain file is definitely required.

2

Also regarding the syntax I think it is better with the following:

[libraries]
lib1.files = [
    'file.vhd',  # Plain file
    'src/*.vhd',  # Pattern without exclude
    {include='tests/*/tb_*.vhd', exclude='*_mock.vhd'}, # Pattern with exclude
]

The reason being that merged config files can be represented. If two config files both have exclude patterns they could not be merged in your original syntax without the exclude in the second file also affecting the files in the first configuration file. We implicitly merge config files since #39 and I want the merged result to be expressible using the configuration file format.

Also using this syntax the exclude pattern can be simple because you know it only applies to the include pattern. When using a pattern without exclude it looks the same as with your proposal.

kraigher avatar Nov 18 '19 18:11 kraigher

1

If a plain file reference is missing then an error is in place. For an include pattern not matching any files I think that a warning is more than enough and for exclude patterns info may be enough.

  • Plain file missing => Error
  • Include pattern without match => Warning
  • Exclude pattern without match => Info

2

I think that both use cases might be valid, however only excluding files from patterns in the "local" configuration is the main use case which makes me lean toward your suggestion.

Bochlin avatar Nov 18 '19 18:11 Bochlin

I think that no error should be shown due to missing files or patterns without match. A warning should be enough in both cases. Errors should be produced if any resource which is expected to be found, is actually not available. This is because a user might want to have multiple libs defined in a config file, but not have all of them cloned in every machine. Producing an error would require to have a different config file for each machine.

umarcor avatar Nov 18 '19 19:11 umarcor

I tend to agree with @umarcor, we should be careful with emitting errors. If a file is missing then the parser will catch the error (assuming the file contents of the missing file is used) and a warning for a missing file from the configuration would then help the user find the problem.

Bochlin avatar Nov 19 '19 08:11 Bochlin

It does not matter so much in practice if it is an error or a warning. The client will only change the color from yellow to red and the prefix from warning to error. We give an error for a missing file today. It does not stop the language server, it just sends a window/showMessage to the client and moves on.

kraigher avatar Nov 19 '19 20:11 kraigher

I added support for glob patterns now. But no exclude pattern yet.

kraigher avatar Nov 21 '19 20:11 kraigher