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

[Feature] CMake Variants from the CMake Tools for Visual Studio Code. #2

Merged
merged 8 commits into from
Oct 30, 2022

Conversation

toolcreator
Copy link
Contributor

Adds support for CMake Variants from the CMake Tools for Visual Studio Code.

It's far from perfect though currently (I'm a Lua beginner and didn't write any comments (yet)).
The YAML parsing is done by simpleyaml which I wrote specifically for this purpose.
At first I couldn't find any simple, dependency-free YAML parsing library. Then I found there are some options on github, but I was already done with my own implementation.
It does only parse well-formed files, probably only parses a subset of YAML, and doesn't have any error handling, so maybe it should be exchanged for something more robust.

A thing I added for this to work more fluently is, that CMakeSelectBuildType deletes the build directory if the chosen build type changes. This way it's quicker to change between build types.

variants.lua contains some CC-BY-SA licensed code from rosettacode.org, but that's compatible with GPLv3.

I'm dropping this here as an idea. If you're interested in integrating the feature we could discuss if we should change anything before merging.
Otherwise I may just keep it as is for my own use.

@Civitasv Civitasv added the enhancement New feature or request label Oct 28, 2022
@Civitasv
Copy link
Owner

Thanks, @toolcreator. It's really a nice feature. Things I think can be discussed:

  1. As you said, you should add some comments.
  2. About parsing YAML, can we just use lua format to store configurations of variants.

@Civitasv
Copy link
Owner

Civitasv commented Oct 28, 2022

  1. About parsing YAML, can we just use lua format to store configurations of variants.

Or support both. And I just check your parser for yaml. I think it's ok, we can make it robust anyway.
What is your opinion?

@toolcreator
Copy link
Contributor Author

toolcreator commented Oct 30, 2022

Thanks for the feedback!

Notable changes

  • Fix luacheck warnings
  • Add comments to both extension code and simpleyaml
  • Catch 1 more error condition in simpleyaml (more details below)

On the input format

I think we should definitely support YAML, because the documentation for the variants makes heavy use of it.
The parsing is still a bit of an issue though: simpleyaml can parse simple and correct files, and from my perspective that's all you need for this feature. Yet, there is no nice feedback to the user in case of an erroneous input file. The extension would just do nothing, or apply a strange setting.
Making simpleyaml more robust would be a bigger task because it is line-based. To detect more errors I think a lot more state variables and/or a switch to character-based parsing would be required.
Unfortunately, it seems there are no(?) alternatives: Searching the web for YAML parsing sooner or later ends up with a YAML binding to some C library, and I wouldn't like adding more dependencies (especially complex non-Lua ones).
Searching for "lua yaml" on GitHub yields a few results that are rather old and also cannot parse the full feature set of YAML, so we might as well stick to my implementation.

The VSCode extension also supports JSON as an input format, and my implementation should do so as well via vim.fn.json_decode. I didn't bother testing this yet (at all) though.

TODO

  • Test JSON input
  • Decide on YAML parsing
  • ...?

@Civitasv
Copy link
Owner

To detect more errors I think a lot more state variables and/or a switch to character-based parsing would be required.

Yeah, I didn't find any lightweight YAML parsing library either. But I recently write a tiny JSON parser in C++. I think it's not that hard to swicth it to character-based parsing. I will try to make one when I have time.

@Civitasv Civitasv marked this pull request as ready for review October 30, 2022 13:03
@Civitasv Civitasv merged commit 92a6b98 into Civitasv:master Oct 30, 2022
@Civitasv Civitasv changed the title Variants [Feature] CMake Variants from the CMake Tools for Visual Studio Code. Oct 30, 2022
@Civitasv
Copy link
Owner

And I think we should also support lua format.

@toolcreator
Copy link
Contributor Author

And I think we should also support lua format.

How would we go about with that? Read it from a file as well? Or do you have someting else in mind?

@Civitasv
Copy link
Owner

Yes, I think it is okay to read it from a file, just like JSON and YAML format. It's natural to me.

Civitasv added a commit that referenced this pull request Jan 7, 2023
Adds support for CMake Variants from the CMake Tools for Visual Studio Code.
Civitasv added a commit that referenced this pull request Jan 7, 2023
Adds support for CMake Variants from the CMake Tools for Visual Studio Code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants