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:corrected pynvim installation instruction in neovim plugin docs #563

Closed
wants to merge 1 commit into from

Conversation

Debajyati
Copy link
Contributor

#562
This PR updates the documentation to provide the correct installation command for pynvim on Debian-based systems. Previously, the documentation suggested using pip install pynvim, which always leads to errors in externally managed environments.

By using sudo apt install python3-pynvim, users can ensure that pynvim is installed correctly and can be used by the plugin.

Copy link

vercel bot commented Aug 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
documentation ✅ Ready (Inspect) Visit Preview Aug 31, 2024 6:55am

@@ -29,7 +29,7 @@ A few things to note before you start installing the Pieces Neovim Plugin:
- The Python based Neovim plugins requires the `pynvim` Python package, which is not included in the default Neovim installation. You can install it by running the following command:

```bash
pip install pynvim
sudo apt install python3-pynvim
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @Debajyati thanks for your contribution, I will recommend that we can add this in an info section in the docs since not everyone will use a Debian based system. For mac users they can still run the current command.

::: info
:::

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I didn't know about mac because I never used one. But I know that it is not just Debian based systems. In any linux distribution you would need to manually install any python package with the system package manager( unless you are inside a virtual environment and adding the libraries as dependency). Like in Arch Linux the command would be - 

sudo pacman -S python3-pynvim

the system package manager would change with the flags/options but the package would always be python3-pynvim
I mentioned Debian Based because Ubuntu is debian based and I read somewhere in the Docs that Pieces currently supports only Ubuntu and all of its derivatives. So all of the supported systems share apt as the package manager. 
And the largest user base of Neovim are Linux Users - 
 

So, hey! @shivay-at-pieces I think that it would be better if we make it a two part section. 
For MacOS users - 

pip install pynvim

If you use Debian/Ubuntu - 

sudo apt install python3-pynvim

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this makes sense @Debajyati

@hra42
Copy link

hra42 commented Sep 5, 2024

@shivay-at-pieces @Debajyati isn't #567 related to that and should be done in one PR?

@Debajyati
Copy link
Contributor Author

I mean I was confused! I asked some people about when someone ( a maintainer ) reviews the PR and requests some changes then should I create a new branch and send a new PR from it or do just the changes in the same branch and send a PR? They suggested me to make a new PR from a fresh new branch. So this is why I did this.

@hra42
Copy link

hra42 commented Sep 5, 2024

@Debajyati ah okay. If the pieces team wants it that way, then it's fine. Just found it confusing if you have multiple pr's for a nearly identical issue.

@Debajyati
Copy link
Contributor Author

Hey just asking, what do of a PR that is not going to get merged?

@hra42
Copy link

hra42 commented Sep 5, 2024

you could close that pr and work on the new pr only. Otherwise about the status we need to wait until the team takes a look at this.

@Debajyati
Copy link
Contributor Author

Ok, thanks. So I will close this PR and make the changes you requested in the new PR I send last time. I will remove Debian from the line I added in the docs in the new PR. Because only Ubuntu 22+ and Ubuntu based distros are supported.

Is that all good!
See I am learning contributions through making contributions. I am not very pro at it. I have read many articles but they only scratch the surface about source control. Real world is much much more to learn.

@Debajyati Debajyati closed this Sep 15, 2024
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.

3 participants