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

Fix/config validation #704

Merged
merged 2 commits into from
Aug 4, 2023
Merged

Conversation

PriceHiller
Copy link
Contributor

This PR ensures the config validation properly passes for mappings that use false to disable them. This PR also adds new tests for this and removes and updates invalid tests to be correct.

@CKolkey

Resolves the new problem from #635

@CKolkey
Copy link
Member

CKolkey commented Aug 4, 2023

Thanks for the quick fix ;)

Lemme know when its done

@CKolkey CKolkey merged commit 8ba948f into NeogitOrg:master Aug 4, 2023
3 checks passed
@PriceHiller
Copy link
Contributor Author

PriceHiller commented Aug 4, 2023

@CKolkey

Revert and let me reopen a new PR, I should have marked this as a draft. There's a minor issue I just discovered causing a full freeze of the Neovim client and somewhat incorrect config validation messages.

If you place an invalid command in currently the config validation commits suicide. I have a fix on my side on a rebase that solves this.

I'll open a separate PR with the small patch to apply.

@CKolkey
Copy link
Member

CKolkey commented Aug 4, 2023

Ah, my bad. I saw the thumbs up as "good to go". But yeah, if you stick to draft/ready convention, thats super easy to follow :)

@PriceHiller
Copy link
Contributor Author

Ah, my bad. I saw the thumbs up as "good to go". But yeah, if you stick to draft/ready convention, thats super easy to follow :)

That's my fault, I thought it was good until I realized at the last moment that it, in fact, was not.

@CKolkey
Copy link
Member

CKolkey commented Aug 4, 2023

Haha, never been there, no sir, can't relate at all.... 🤣🤣

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.

2 participants