windwp / nvim-ts-autotag

Use treesitter to auto close and auto rename html tag
MIT License
1.54k stars 85 forks source link

Add a simple minimum repro for issues (create a template) #188

Open PriceHiller opened 1 month ago

PriceHiller commented 1 month ago

To cut down on issues opening with "x thing doesn't work" and similar explanations, it would be good to add a simple issue template with a minimal init to demonstrate the issue.

The minimal init for the tests is a good candidate for the minimal init template.

roycrippen4 commented 5 days ago

Here are the essentials for a bug report with repro (shamelessly taken from lazy.nvim).

NOTE: Comments regarding the form will be marked using --[[comment]]

Required fields are marked with *

Input fields are marked with _.

Add a title

bug: _

Before reporting an issue, make sure to read the documentation and check for existing issues.

Usage questions such as "How do I...?" belong in [discussions]() and will be closed. --[[ Not sure if this is needed. Discussions will need to be configured if this is needed.]]

Did you check docs and existing issues?

Make sure you check all boxes below before submitting an issue

Neovim version (nvim -v) *

_

Operating system/version *

_

Describe the bug *

A clear and concise description of what the bug is. Please include any related errors you see in Neovim.

_

Steps to Reproduce *

_

Expected Behavior *

A concise description of what you expected to happen.

_

Repro

Minimal init.lua to reproduce this issue. Save as repro.lua and run with nvim -u repro.lua

vim.env.LAZY_STDPATH = ".repro"
load(vim.fn.system("curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua"))()

require("lazy.minit").repro({
  spec = {
        "windwp/nvim-ts-autotag",
        -- other related plugins here
  },
})

--[[ The repro code should autofill the repro text field. A user is expected to include any related plugins underneath nvim-ts-autotag. I put the repro code in a fenced code block for purely legibility in this comment. Check here and click Get started on Bug Report as a reference. ]]

PriceHiller commented 3 days ago

Here are the essentials for a bug report with repro (shamelessly taken from lazy.nvim).

I actually like the issue template that Lazy uses, wouldn't mind nabbing it.

The only thing I am iffy on is using Lazy's minimum repro code. Ideally the minimal init we use in our tests is identical (or very close) to the one we use in the issue template.

If you want to PR this, I'm totally open to looking at what you throw my/our way. The primary concern I have with using Lazy's minimum repro is the lack of fine control over the initialization and the fact that by explicitly defining just about everything ourselves we get a really good idea of the absolute minimum nvim-ts-autotag requires to operate.

roycrippen4 commented 3 days ago

I just tried out running nvim -u ./tests/minimal_init.lua and was able to get nvim to run properly with nvim-ts-autotag working and everything, which is good.

I'm not sure what the best solution would be. I could argue for using the lazy repro or a modified version of the testing config.

The min repro should probably have the following characteristics:

  1. Simple and easy to use The repro should require little to no Neovim/Lua experience. Users shouldn't be bogged down with a complicated repro process when they're trying to post an issue. Adding friction to the issue process lowers the quality of issues reported.

  2. As controlled as possible Minimize variables to improve likelihood of reproducing the bug. The goal is to narrow down the source of the issue to resolve it quickly and effectively.

  3. Include other plugins. Users can't be expected to give a repro for an issue that relates to another plugin if a mechanism for adding other plugins isn't implemented. Inability to include other plugins puts the burden reproduction back onto the maintainers.

The good news is that it looks like minimal_init.lua checks all of those boxes. Maybe there's a way to adapt the concepts that lazy.nvim is using into something that works for this project. Just need to figure out how to apply those ideas.

The nice thing about the lazy.nvim repro is just how low that fruit is hanging haha

PriceHiller commented 2 days ago

I agree with all three points you raised above.

So here's the deal, instead of clutching my pearls on "what ifs", if you send a PR using a lazy.nvim repro code I'm in. Just make sure the minimal init in the tests is refactored to use it as well.

One of big the things from our current minimal init is that we don't really mess with any vim.opt stuff besides the runtimepath and the packpath. Ideally we'd keep that, because it makes our minimal init closely mirror a real configuration a user may have.

The minimal init we provide to users in issues should be the same as the one in the tests. The reason I'm adamant on that point is that it means we regularly exercise the minimal init so there's no doubts as to where an issue may be.

I think there's a good argument to use the lazy.nvim minimal init: the larger community is likely to be using it as well so it'll get the fixes from everyone involved instead of us all rolling our bespoke minimal inits.

TL;DR

If you give me a PR with a lazy.nvim minimal init I'm good with it, just make sure it satisfies the following:

If you want to break this into two PRs, that's ok as well. One for the update to the tests minimal init and one for the actual issue template.