Skip to content

Conversation

@AgatZan
Copy link
Contributor

@AgatZan AgatZan commented Aug 15, 2024

  • chore: added typing
  • fix(wildcard): bugfix wildcard.parse. When finding a wildcard at lookup, it doesn't check if it function or string.
  • fix(util): probably bugfix. The default value for the inserttable pattern is nil. If gettemplates encounters nil, it will not process it.
  • feat(ui): Adding probability to change the default UI when selecting a template process. ( async or not)

-- ---@param dir string
-- ---@param ignored_patterns string[]
-- ---@return string[]
-- local listignore = function(dir, ignored_patterns)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Iter alternative to listignored

Copy link
Owner

Choose a reason for hiding this comment

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

I would add this version but I need to implement proper testing before relying on changing it. Will test it in my local version and decide on wheter we use this one for now.

@cvigilv
Copy link
Owner

cvigilv commented Aug 15, 2024

Awesome PR! Just a couple of questions:

  1. Why did you revert the CI implementation?
  2. Could you provide an example of a custom selector in the README? or even here, so I can test it works properly.
  3. With the provided changes, I'm unable to make the plugin work. Could you provide your configuration to test it?

Thanks for the awesome work, I was aiming to implementing stuff like this in a near future whenever I was a little more free from work.

@AgatZan
Copy link
Contributor Author

AgatZan commented Aug 15, 2024

Awesome PR! Just a couple of questions:

  1. Why did you revert the CI implementation?
  2. Could you provide an example of a custom selector in the README? or even here, so I can test it works properly.
  3. With the provided changes, I'm unable to make the plugin work. Could you provide your configuration to test it?

Thanks for the awesome work, I was aiming to implementing stuff like this in a near future whenever I was a little more free from work.

  1. Because I was trying to figure it out during the work period, but since it fell with every commit, I decided that I still need to work on it.

In general, in order to start covering the repository with tests

  1. I added selector to esqueleto/selector/builtin
function(templatenames)
	-- Sync call ui.select
	-- See: https://github.com/mfussenegger/nvim-dap/blob/66d33b7585b42b7eac20559f1551524287ded353/lua/dap/ui.lua#L55
	local co = coroutine.running()
	local choicer = function(choice)
		if not choice then
			vim.notify(
				"[esqueleto] No template selected, leaving buffer empty",
				vim.log.levels.INFO
			)
		end
		coroutine.resume(co, choice)
	end
	-- I don't know reason to use that
	-- choicer = vim.schedule_wrap(choicer)
	vim.ui.select(templatenames, { prompt = "Select skeleton to use:" }, choicer)
	return coroutine.yield()
end

I took the very idea of implementing this from here

  1. I got a little carried away with my study of the codebase through adding types and missed this point, so most likely there will be bugs. I apologize m(_ _)m

nevertheless, after checking, I have it open on simple files

my config

@cvigilv
Copy link
Owner

cvigilv commented Aug 15, 2024

  1. Perfect, I will do some work now dedicated to CI/CD so I can release a new version.

  2. I noticed it, just thought you may have implemented something custom on your config (like a Telescope selector or something like that). I'll give it a go to implement new selectors locally, but for now this works as intended.

Regarding point 3. I made a mistake on my side, that's why it didn't work. It works as intended so, don't worry.

Overall the PR is great, just some stylistic differences that can be revised. Will merge on the weekend after going through everything in the case I need some changes. Thanks!

@cvigilv cvigilv changed the title Feat: Adding the ability to change the UI of the templatename selection feat(ui): add ability to change UI of template selection Aug 16, 2024
@cvigilv
Copy link
Owner

cvigilv commented Sep 25, 2024

Haven't merged this since its not working on my local version. Will try to see how I can incorporate this in the coming version. For now I will let it as is and get back to it whenever I finish the other things I'm developing for the plugin.

@cvigilv
Copy link
Owner

cvigilv commented Oct 2, 2024

Could you provide a screen recording of this working? havent been able to make it work in my local version

@AgatZan AgatZan mentioned this pull request Oct 2, 2024
@AgatZan
Copy link
Contributor Author

AgatZan commented Oct 2, 2024

Could you provide a screen recording of this working? havent been able to make it work in my local version

plugin/esqueleto.lua

return {
	"AgatZan/esqueleto.nvim",
	---@type Esqulato.config
	opts = {
		patterns = { "python" },
	},
}

It`s test builtin selector
neovide_790IeOIcca

@AgatZan
Copy link
Contributor Author

AgatZan commented Oct 2, 2024

Also work with esqueleto.selectors.telescope but if you use lazy this may be obvious, but at opts selector=require("esqueleto.selectors.telescope") throw error, so configurate at config = function()
image

@cvigilv
Copy link
Owner

cvigilv commented Oct 2, 2024

It indeeds works, dont know why it would not work on my computer. What version of Neovim are you using? I generally work in nightly for plugin dev.

@AgatZan
Copy link
Contributor Author

AgatZan commented Oct 2, 2024

NVIM v0.10.1
Build type: Release
LuaJIT 2.1.1713484068


-- wrap to coroutine to have possibility to async function
---@see esqueleto.selectors.builtin
coroutine.wrap(function()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think only one breaking chang at that place, if it work correct, then it must work correct either errors exists at another functions

Copy link
Owner

Choose a reason for hiding this comment

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

Using this PR, I get the following error:

Error detected while processing BufNewFile Autocommands for "*":
Error executing lua callback: .../latest/nvim-macos-arm64/share/nvim/runtime/filetype.lua:36: BufNewFile Autocommands for "*"..FileType Autocommands for "*"..FileType Autocommands for "*"..FileType Autocomman
ds for "*"..FileType Autocommands for "*"..FileType Autocommands for "*"..FileType Autocommands for "*"..FileType Autocommands for "*"..FileType Autocommands for "*"..FileType Autocommands for "*": Vim(append
):Error executing lua callback: ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:182: ...queleto_dev/lazy/plenary.nvim/lua/plenary/popup/init.lua:122: Vim:E218: Autocommand nesting too deep
stack traceback:
        [builtin#36]: at 0x01008f59f0
        ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:182: in function 'selecttemplate'
        ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:208: in function 'inserttemplate'
        ...carlosvigil/git/esqueleto.nvim/lua/esqueleto/autocmd.lua:19: in function <...carlosvigil/git/esqueleto.nvim/lua/esqueleto/autocmd.lua:15>
        [builtin#36]: at 0x01008f59f0
        ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:182: in function 'selecttemplate'
        ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:208: in function 'inserttemplate'
        ...carlosvigil/git/esqueleto.nvim/lua/esqueleto/autocmd.lua:19: in function <...carlosvigil/git/esqueleto.nvim/lua/esqueleto/autocmd.lua:15>
        [builtin#36]: at 0x01008f59f0
        ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:182: in function 'selecttemplate'
        ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:208: in function 'inserttemplate'
        ...
        [builtin#36]: at 0x01008f59f0
        ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:182: in function 'selecttemplate'
        ...s/carlosvigil/git/esqueleto.nvim/lua/esqueleto/utils.lua:208: in function 'inserttemplate'
        ...carlosvigil/git/esqueleto.nvim/lua/esqueleto/autocmd.lua:19: in function <...carlosvigil/git/esqueleto.nvim/lua/esqueleto/autocmd.lua:15>
        [C]: in function 'nvim_cmd'
        .../latest/nvim-macos-arm64/share/nvim/runtime/filetype.lua:36: in function <.../latest/nvim-macos-arm64/share/nvim/runtime/filetype.lua:35>
        [C]: in function 'pcall'
        vim/shared.lua: in function <vim/shared.lua:0>
        [C]: in function '_with'
        .../latest/nvim-macos-arm64/share/nvim/runtime/filetype.lua:35: in function <.../latest/nvim-macos-arm64/share/nvim/runtime/filetype.lua:10>
stack traceback:
        [C]: in function '_with'
        .../latest/nvim-macos-arm64/share/nvim/runtime/filetype.lua:35: in function <.../latest/nvim-macos-arm64/share/nvim/runtime/filetype.lua:10>

@AgatZan
Copy link
Contributor Author

AgatZan commented Oct 2, 2024

I test it with minimal-minimal setup.

DEV_DIR = "~/nvim-plug/esqueleto.nvim"
vim.opt.rtp:prepend(DEV_DIR)
require("esqueleto").setup({patterns = {"python"}}

And only 1 that i found, that i should decorate if selectors/builtin.lua choicer

@cvigilv
Copy link
Owner

cvigilv commented Oct 2, 2024

I didnt get what you refer to. I prefer you test things with the lazy-based minimal example since the vast majority of user use lazy for pkg management.

@AgatZan
Copy link
Contributor Author

AgatZan commented Oct 2, 2024

I think it doesn't really matter, but i test and it works

neovide_XdYvcNjLYB

@cvigilv
Copy link
Owner

cvigilv commented Oct 2, 2024

Are you cleaning the plugins whenever you launch neovim? I'm using CLI neovim and use the NVIM_APPIMAGE environmental variable to ensure no other plugins are being loaded. I don't know if there is an equivalent in Neovide.

@AgatZan
Copy link
Contributor Author

AgatZan commented Oct 2, 2024

I removed all cached plugins and used a minimal setup and use cli nvim. I had no problems.

@cvigilv
Copy link
Owner

cvigilv commented Oct 3, 2024

Lost for words here. Will have to deep dive into telescope to see what is the problem. Thanks for your help

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.

2 participants