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

feat: Config builder #163

Merged
merged 4 commits into from
Nov 5, 2024
Merged

Conversation

WillLillis
Copy link
Collaborator

@WillLillis WillLillis commented Oct 27, 2024

I am greatly excited by the config improvements coming from #158. These changes do make the config a bit more complex though, which may be confusing/hard to track in large projects/ for new users. This PR adds a small CLI program that prompts a user through creating a .asm-lsp.toml config file, which should help manage that complexity. I'll add some more details to this PR description (and some example gifs) once it's closer to being merged, but I wanted to open this up early for the sake of visibility.

Currently, the config builder program lives as a separate binary crate within the project. I think I'd like to change this to run as a subcommand of the main program (assuming it doesn't bloat the binary size by too much). Some documentation for this also needs to be added to the project's README.

Here's an example gif. I don't currently have a large assembly project to demonstrate this on, so I'm just going to run the tool within the asm-lsp repo selecting some nonsense options:

ConfigBuilder

(The flickering only shows up in the recording, I'm not entirely sure what's causing that)

The builder provides validation for selected options in the following ways:

  • Any enum field specified (i.e. arch or assembler) will be valid
  • project paths will point to valid directories or files on the system. After selection, the builder translates the absolute path to one relative to the project's root directory (for portability reasons)
  • For any compiler specified, a check is made to ensure it is a valid path to an executable file, or that it can be found on $PATH.
  • Project configs are checked for redundant paths

Closes #161

asm-lsp-cfg/src/main.rs Outdated Show resolved Hide resolved
asm-lsp-cfg/src/main.rs Outdated Show resolved Hide resolved
asm-lsp-cfg/src/main.rs Outdated Show resolved Hide resolved
@WillLillis WillLillis force-pushed the config_builder branch 7 times, most recently from e051a5a to 193aba6 Compare October 30, 2024 05:48
@WillLillis
Copy link
Collaborator Author

WillLillis commented Oct 30, 2024

@lu-zero I'm curious how well this fits in with your use case for dav1d (very cool project by the way!). If you have the time to test this out, I'd really like to address any issues/pain points you run into. :)

EDIT: Not sure why CI isn't running now, but at this point I think that's a tomorrow (later today) problem.

@WillLillis WillLillis force-pushed the config_builder branch 9 times, most recently from f0c9553 to 3cc9614 Compare November 3, 2024 22:19
@WillLillis WillLillis marked this pull request as ready for review November 3, 2024 22:20
@WillLillis
Copy link
Collaborator Author

Rebased and added some checks to ensure multiple projects couldn't have the same path. I'm planning on merging this in tomorrow night unless I spot some other issues.

@Ultra-Code
Copy link
Contributor

Looks good, Impressive work 🔥 🚀 .

@WillLillis WillLillis force-pushed the config_builder branch 2 times, most recently from b11d07a to 76651bb Compare November 5, 2024 00:33
@WillLillis WillLillis enabled auto-merge (squash) November 5, 2024 00:34
@WillLillis WillLillis disabled auto-merge November 5, 2024 00:34
WillLillis and others added 4 commits November 4, 2024 19:47
* feat: start selecting project paths from the output_dir

* feat: Sort the entries, directories first
@WillLillis WillLillis merged commit 712c9e6 into bergercookie:master Nov 5, 2024
15 checks passed
@WillLillis WillLillis deleted the config_builder branch November 5, 2024 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config Generator
3 participants