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

Add necessary bits to install and enable copilot. #18833

Merged
merged 38 commits into from
Oct 17, 2022
Merged

Conversation

georgalis
Copy link
Contributor

@georgalis georgalis commented Jun 28, 2022

No need to send users down the rabbit hole of how many legacy ways to install vim plugins... Simply direct them to use the new builtin nvim plugin support.

Why:

Closes 19579

What's being changed (if available, include any code snippets, screenshots, or gifs):

add necessary steps to docs

Check off the following:

  • I have reviewed my changes in staging (look for the "Automatically generated comment" and click the links in the "Preview" column to view your latest changes).
  • For content changes, I have completed the self-review checklist.

Writer impact (This section is for GitHub staff members only):

  • This pull request impacts the contribution experience
    • I have added the 'writer impact' label
    • I have added a description and/or a video demo of the changes below (e.g. a "before and after video")

No need to send users down the rabbit hole of how many legacy ways to install vim plugins... Simply direct them to use the new builtin nvim plugin support.
@welcome
Copy link

welcome bot commented Jun 28, 2022

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@georgalis georgalis temporarily deployed to preview-env-18833 June 28, 2022 02:11 Inactive
@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Jun 28, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 28, 2022

Automatically generated comment ℹ️

This comment is automatically generated and will be overwritten every time changes are committed to this branch.

The table contains an overview of files in the content directory that have been changed in this pull request. It's provided to make it easy to review your changes on the staging site. Please note that changes to the data directory will not show up in this table.


Content directory changes

You may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.

Source Preview Production What Changed
copilot/getting-started-with-github-copilot/getting-started-with-github-copilot-in-neovim.md fpt
ghec
fpt
ghec

fpt: Free, Pro, Team
ghec: GitHub Enterprise Cloud
ghes: GitHub Enterprise Server
ghae: GitHub AE

@georgalis georgalis temporarily deployed to preview-env-18833 June 28, 2022 02:19 Inactive
@georgalis
Copy link
Contributor Author

Is the failing test working properly? I forked the docs repo to create a pull request which does not contain any new repo reference and my fork is public https://github.com/georgalis/docs yet,

FAIL tests/meta/repository-references.js (17.464 s, 537 MB heap size)
  ● check if a GitHub-owned private repository is referenced › in file content/copilot/getting-started-with-github-copilot/getting-started-with-github-copilot-in-neovim.md

what repository reference does the failing test refer to?

adjust referenced repo from
```
https://github.com/github/copilot.vim.git
```
to
```
https://github.com/github/copilot.vim
```
I'm not sure this makes a difference with regard to `private repositories` if not, perhaps the test should check PUBLIC_REPOS with .git as valid.
@georgalis georgalis temporarily deployed to preview-env-18833 June 28, 2022 05:41 Inactive
@janiceilene
Copy link
Contributor

@georgalis Thanks so much for opening a PR! I'll get this triaged for review ✨

@janiceilene janiceilene added content This issue or pull request belongs to the Docs Content team waiting for review Issue/PR is waiting for a writer's review copilot Content related to GitHub Copilot and removed triage Do not begin working on this issue until triaged by the team labels Jun 29, 2022
djensenius
djensenius previously approved these changes Jul 27, 2022
Copy link

@djensenius djensenius left a comment

Choose a reason for hiding this comment

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

I didn't even realize nvim has a built in plugin manager. I think most folks use other plugin managers, but this looks correct.

It might be worth saying something like "use your plugin manager to install copilot or to use the built in plugin manager", as I was super confused at first.

@georgalis georgalis temporarily deployed to preview-env-18833 July 27, 2022 15:37 Inactive
@georgalis
Copy link
Contributor Author

@djensenius @janiceilene just curious, this documentation fixup, and revision per feedback, has been open over 5 weeks? seems a long time for low hanging fruit?

@cmwilson21
Copy link
Contributor

@georgalis Thanks for checking in on this one. Your PR is on the board and waiting for review. A writer will get eyes on it soon. 👀

We appreciate your patience as we catch up on the summer backlog. 💛

@jules-p jules-p temporarily deployed to preview-env-18833 August 9, 2022 14:17 Inactive
@jules-p jules-p temporarily deployed to preview-env-18833 August 9, 2022 14:38 Inactive
@jules-p
Copy link
Contributor

jules-p commented Aug 9, 2022

Hi @georgalis 👋🏻 I've made a couple of small changes to simplify the language and apply the changes across all 3 operating systems. I've got a follow up PR for the install-copilot-in-neovim reusable, which - once it's merged - will read:

  1. To use {% data variables.product.prodname_copilot %} in Neovim, install the {% data variables.product.prodname_copilot %} plugin. You can either install the plugin from a third party plugin manager or from Neovim's built in plugin manager. For more information on third party plugin managers, like vim-plug or packer.nvim, see the documentation for the plugin manager. For example, you can see the documentation for vim-plug or packer.nvim.

Which will then be followed by your updated installation step:

  • To install {% data variables.product.prodname_copilot %} with a plugin manager, enter the following command in Terminal.
  ```
  mkdir -p ~/.config/nvim/pack/github/start
  git clone https://github.com/github/copilot.vim \
           ~/.config/nvim/pack/github/start/copilot.vim
  ```

If that sounds okay to you, I'll go ahead and get those changes merged?

@georgalis
Copy link
Contributor Author

Hi @jules-p that explicitly reverts the key language of my change. Neovim has a built in plugin manager. Keep it simple, use that, and indicate alternate managers will also work if they are preferred.

If you suggest a 3rd party plugin manager first, anyone that doesn't know how to use one already will research and find a ton of options and began an evaluation process. Which one is best? How best to implement? All the integration options don't help new users. Fine to keep that door open but there is no reason to send new users down that road.

The neovim solves the world of vim plugin manages with a built in one. There is no need to introduce new copilot users to legacy plugin manager options of vim.

Guide new users onto the simple path. If they want a different plugin manager, let it be their prerogative.

@jules-p
Copy link
Contributor

jules-p commented Aug 11, 2022

Hi @georgalis, I see what you're saying. I'm happy to highlight the built-in plug-in manager first, although I don't think we want to entirely obscure the other paths. Would something like this 👇🏻 address your concerns?
neovim-install

@jules-p
Copy link
Contributor

jules-p commented Aug 24, 2022

Hi @georgalis, I've commited the necessary changes, except for the updates to the {% data reusables.copilot.install-copilot-in-neovim %}, which I can't access in your fork, and which currently contains the introduction to the first step, and the first sub bullet point (which is no longer required). If you could please update this file, replacing the current content with the following content, that would be great:

1. {% data variables.product.prodname_dotcom %} recommends that you install the {% data variables.product.prodname_copilot %} plugin through Neovim’s built-in plugin manager. Alternatively, you can use another plugin manager, or you can install the plugin directly.

If you look at the article in staging you'll see that it has automatically detected the code block, and the fence posts are not required in this instance.

@georgalis
Copy link
Contributor Author

Hi @jules-p, thanks for clearing that up. Hopefully this refactor will cover all the bases. In the macro you asked about, I removed specific plugin managers (search for best vim plugin manager for why), and added another macro for the config/enable text. 🎉

@jules-p jules-p self-assigned this Sep 14, 2022
git clone https://github.com/github/copilot.vim \
~/.config/PATH/TO/YOUR/NEOVIM/CONFIG/FILE/copilot.vim
1. To configure {% data variables.product.prodname_copilot %}, open Neovim and enter the following command.
mkdir -p ~/.config/nvim/pack/github/start
Copy link

Choose a reason for hiding this comment

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

git clone does an implicit mkdir -p, so this command is unnecessary.

Comment on lines +60 to +61
git clone https://github.com/github/copilot.vim \
~/.config/nvim/pack/github/start/copilot.vim
Copy link

Choose a reason for hiding this comment

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

The correct path on Windows is ~/AppData/Local/nvim/pack/github/start/copilot.vim. Unfortunately it's not quite a drop-in replacement, as ~ and \ are specific to UNIX shells. In the copilot.vim README, I'm currently suggesting the following PowerShell command:

          git clone https://github.com/github/copilot.vim.git `
            $HOME/AppData/Local/nvim/pack/github/start/copilot.vim

@tpope
Copy link

tpope commented Sep 14, 2022

No need to send users down the rabbit hole of how many legacy ways to install vim plugins... Simply direct them to use the new builtin nvim plugin support.

I'd be remiss not to point out most users do not consider plugin managers to be legacy. They handle a lot more than the Neovim built-in—namely installation and upgrading—and indeed some of them were created after the built-in support was added. But I think the one sentence acknowledgement here is sufficient.

Note that since the creation of this PR, copilot.vim gained support for vanilla Vim in addition to Vim, and that necessitates the use of different installation paths. But I wouldn't let that delay shipping this; we can circle back on that later.

Copy link
Contributor Author

@georgalis georgalis left a comment

Choose a reason for hiding this comment

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

If that works, it's fine with me.

@jules-p jules-p self-requested a review October 17, 2022 10:59
@jules-p jules-p merged commit e364397 into github:main Oct 17, 2022
@github-actions
Copy link
Contributor

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content This issue or pull request belongs to the Docs Content team copilot Content related to GitHub Copilot waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants