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

Updated readme to show that setup is required for plugin to work correctly. #5

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

linguini1
Copy link
Owner

I should have included that the setup() call to pulse.nvim was required to get access to editor commands. This documentation patch clarifies that and shows the base install configuration.

This closes #4.

@linguini1 linguini1 added the documentation Improvements or additions to documentation label Oct 20, 2023
@linguini1 linguini1 self-assigned this Oct 20, 2023
Copy link

@Normanras Normanras left a comment

Choose a reason for hiding this comment

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

On line 36, you mention pulse.setup() but your config examples above doesn't set pulse as a local variable. I would keep it consistent since anything below that sets a local pulse variable.

This might be just me, but so many of my other plugin configs get entered into a table in those same setup parentheses. I'd consider adding to line 51 something that says you can add pulse.add() below/outside the closed setup parenthesis.

@linguini1
Copy link
Owner Author

@Normanras could you take another look and see if this is clearer?

Copy link

@Normanras Normanras left a comment

Choose a reason for hiding this comment

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

Looks good! Much clearer and simple.

Copy link

@Normanras Normanras left a comment

Choose a reason for hiding this comment

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

No changes from the guy who opened the issue originally. Thanks for a great plugin!

@linguini1 linguini1 merged commit 0b5ab16 into main Oct 20, 2023
6 checks passed
@linguini1 linguini1 deleted the fix-installation-guide branch October 20, 2023 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installed, but no commands?
2 participants