Skip to content

Conversation

@D7ry
Copy link
Contributor

@D7ry D7ry commented Mar 26, 2024

By configuring notify_done variable in opts, users can disable the "done!" notification that some may find a bit distracting.

@jellydn jellydn requested a review from deathbeam March 26, 2024 04:12
@jellydn jellydn added the good first issue Good for newcomers label Mar 26, 2024
@deathbeam
Copy link
Collaborator

deathbeam commented Mar 26, 2024

I think way better way would be to remove the notify completely from where its being called now and just set default callback (thats currently by default nil) to be function that calls vim.notify. And then you can disable it by passing empty function instead or just false (even though that do not matches with the config spec so would recommend empty function instead)

Copy link
Collaborator

@deathbeam deathbeam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As i said in my response, should just be callback imo

@D7ry
Copy link
Contributor Author

D7ry commented Mar 27, 2024

As i said in my response, should just be callback imo

I believe just not setting the default callback is better; removed the notify completely!

@deathbeam
Copy link
Collaborator

Yea i agree, much cleaner solution

Copy link
Collaborator

@deathbeam deathbeam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

@jellydn jellydn requested a review from deathbeam March 27, 2024 09:23
@jellydn jellydn changed the title Feat: add configurable option to disable "done!" notification Remove "done!" notification Mar 27, 2024
@jellydn jellydn merged commit 367f845 into CopilotC-Nvim:canary Mar 27, 2024
@jellydn
Copy link
Contributor

jellydn commented Mar 27, 2024

@all-contributors add @D7ry for code.

@allcontributors
Copy link
Contributor

@jellydn

I've put up a pull request to add @D7ry! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants