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

Chat about ideas #2

Open
derekthecool opened this issue Apr 24, 2022 · 20 comments
Open

Chat about ideas #2

derekthecool opened this issue Apr 24, 2022 · 20 comments

Comments

@derekthecool
Copy link
Contributor

This is an amazing project you've created!

It totally was something I've been looking for with my new dap setup. I've created a small pull request #1

I also wanted to chat about some ideas for development.
One thing I would like to help work on getting telescope to show some of the args for each of the configs.

What are there any other ideas you have?

@kndndrj
Copy link
Owner

kndndrj commented Apr 24, 2022

Hi, thanks for showing interest in my project!

I haven't really had the propper time to work on this plugin for quite some time, but I am open for Ideas! If you have any suggestions I'll be glad to listen!
I have some local changes that I haven't yet pushed (it's about sorting configs in telescope view) - I'll try to push that asap.
Other than that I thought that this plugin could be some kind of a "combined config for other plugins" - so that it wouldn't be nvim-dap specific (I would be glad to hear your thoughts about that :)).

I guess that if we go into that direction, we can also rename the pluggin from nvim-dap-projector to nvim-projector, do you agree?
Also I guess it would be nice if we add this plugin to the nvim-dap wiki, so other people can find this plugin more easily.

@kndndrj
Copy link
Owner

kndndrj commented Apr 24, 2022

Oh, and another thing - I'll create a development branch, where we can test PRs before we merge them to master :)

@derekthecool
Copy link
Contributor Author

Yeah the plugin name change would make sense since you it is not limited to dap.

Dap is just of course very important for projects and benefits from this plugin.

Development branch sounds fine!

@kndndrj
Copy link
Owner

kndndrj commented Apr 25, 2022

Fine, I'll change the name then!
Also, the development branch is up - If you are testing some stuff, you can probably switch to it with the pluggin manager (e.g. packer)

@derekthecool
Copy link
Contributor Author

Hi @kndndrj , I have the new version and have updated my fork with the new name.

I thought of another thing to ask you about.
What do you think about moving the project file from json to lua?
I would love to have luas functions to help with project config?

Though of course it comes with pros and cons and json is already working.

@kndndrj
Copy link
Owner

kndndrj commented May 28, 2022

Hey, sorry for the really late reply.

To answer your last question... Sorucing lua files is not the way to go in my opinion. it seems a bit unsafe to Source a random lua file on vim's startup. and I also think it's a bit overkill for this plugin.

One thing that I have given some thought though is to also accept yaml files. But the main problem here is that lua supplied by neovim does support json out of the box, but it doesn't support yaml.
I tried to pipe the yaml file through yq, but It seems quite a messy solution, so I abandoned it :)

@derekthecool
Copy link
Contributor Author

Okay then it seems like json is still the best bet.

I think it would be good to address the error messages that get thrown when a json format error occurs.
When I have gotten error messages it seems as if my neovim config is the problem and not my local file.

Btw, are we allowed to put comments into the json file?

@kndndrj
Copy link
Owner

kndndrj commented Aug 25, 2022

Yeah, the error handling leaves much to be desired, I agree. If you have some time, you can go through the code and check it.
I'd do it myself, but I've been a bit busy lately :).

Comments are permitted in json, yeah.

@kndndrj
Copy link
Owner

kndndrj commented Sep 29, 2022

By the way, I'm trying to rework the plugin a little. Make it more user friendly and simplify the codebase. Once it's done, I'd be glad to have some help with testing!

@derekthecool
Copy link
Contributor Author

Awesome that sounds good! I've found a bigger and bigger need for this plugin and would love to help keep working on it.

Here are some additional features I've been thinking about:

  • Extra validation to make sure the program file used in debug exists

  • Format the telescope viewer to have more space to view more of the debug item name. I have some long test names and could use more than 19 chars length.
    image

  • If no json config is found, build one! It just opens a blank launcher when there are none found like this:
    image

  • If there is only one item in the task list, allow a way auto launch it without using telescope. This may come down to personal preference so it may need to be configurable somehow. I've got a project right now where I just have one debug item and I'm finding that additional enter press from telescope annoying.

  • Building from the last idea, it may be nice to have a method to insert a new task item. Perhaps a rough idea would look work like this:

    Take input say from vim.ui.select first to ask which type of item to add (debug, task, database...). Next filter again, for debug I'm thinking to list all the languages you already have in your json file. So for example listing create new (c, bash, netcoredbg... etc) + create new item.

Next some other code items I'd like to add would be:

  • Stylua project code formatting. This is an easy addition, create simple config file named stylua.toml and then rely on auto formatting.
  • Plenary tests. Setting up these tests is very easy! I'd like to start adding some right away if you're okay with it.

@kndndrj
Copy link
Owner

kndndrj commented Oct 1, 2022

Oh man, that's AWESOME!!
First off, I like pretty much all your suggestions. some implementation details are of course debatable, but thats beside the point.

However, as I mentioned, I have been working on a rework for some time now and am really excited to share it once some rough edges are polished.

To give you a glimpse: I made the plugin basically modular, which means that I basically provide the core and then other people (and me) can write:

  • "loaders" (basically functions that load configs - e. g. loader for tasks.json, for cargo's tasks, for package.json etc.)
  • "outputs" or runners, that just implement a few methods for running the task (e. g. get the config and run it) - there can be outputs for debug, task or database (maybe more) so an output just basically runs the command (its super simple i just can't explain it well)

But with this model I got some very nice stuff working! For example toggling dapui and projectors terminal with the same keymap.

Another thing: I'm thinking about ditching telescope and using builtin selector (vim.ui.select()), as telescope can easily be added via something like dressing.nvim (I haven't decided yet, but current telescope implementation is a bit messy and I dont like it that much)

last thing: alongside stylua and tests, I will also add docstrings.

So, please wait just a bit more, so I push the changes to github before putting too much work into this project :)

Thanks for the suggestions btw, really appretiate it!

@derekthecool
Copy link
Contributor Author

@kndndrj , I am excited for the updates! I am especially excited for the loader feature!

I have never heard of dressing.nvim yet. I have been using this telescope extension which is actually listed as inspiration by dressing.

Take as much time as you need for the updates. I'll be ready to help when you're done!

@kndndrj
Copy link
Owner

kndndrj commented Oct 12, 2022

@derekthecool You can check the refactor branch now! Let me know what you think.

@derekthecool
Copy link
Contributor Author

Amazing!! 😄

I'll start using this right away. I'll share feedback soon after I've had more time to work with it.

@kndndrj kndndrj pinned this issue Oct 13, 2022
@derekthecool
Copy link
Contributor Author

Hi kndndrj. I've had such a busy couple of days. But I wanted to just say that I have been using it and it is amazing!

I'm still testing the new features. Great job an the refactor, this is so cool!
I'm planning on a much more in-depth review followed by help find and is address any issues.

@derekthecool
Copy link
Contributor Author

Okay, I've found some time today to review more.

Let me share my thoughts of the things I love and some of the issues I've found.

First things that I love:

  • Your design changes are amazing and I love the loaders! Soon I have several loaders I'd like to design. Which makes me think that soon we'll need a wiki for this repo. There are so many ways to customize this!
  • Next I love the notifications and awesome helper features like helper to convert legacy config and showing when config file can't be parsed.
  • Also, I just barely after years of developing software just started using databases. So I took some time to try that functionality out as well. It works so well and I think you're a genius!
  • Plugin is working %100 perfectly with Linux and Windows for me. So great job creating this in a cross platform manner!'ll

And items I've found issues with so far:

  • I found some issue with the getting the cwd of the loader working the way I would expect.
    I use the plugin vim-rooter and it helps to set cwd properly.
    In my testing I found that if I opened the neovim like this nvim directory/Program.cs, the loader looks for my file in that same directory instead of the updated cwd from vim-rooter.

I have the builtin loader like this:

  loaders = {
    {
      module = 'builtin',
      opt = vim.fn.getcwd() .. OS.separator .. 'projector.json',
    },

Command I used to open neovim

cd C:\Users\Derek Lomax\source\repos\DatabaseTesting\Dapper' # should be my root directory
nvim Program.cs

Debugging log message that I used to find this issue in lua/projector/loaders/builtin.lua

    local path = opt or (vim.fn.getcwd() .. "/.vim/projector.json")
    if not vim.loop.fs_stat(path) then
      vim.notify('Error file is not found', vim.log.levels.ERROR, { title = 'nvim-projector' })
      vim.notify(path, vim.log.levels.ERROR, { title = 'nvim-projector' })
      return
    end

Notification log show this:

2022-10-16T17:19:25 nvim-projector  ERROR Error file is not found
2022-10-16T17:19:25 nvim-projector  ERROR C:\Users\Derek Lomax\source\repos\DatabaseTesting\Dapper\projector.json

This would seem to be caused by calculating the cwd right when neovim is opened.

This is not a bug or a bad feature, it is simply different from what I'd expect. Also, it is different from how it was handled in the plugin prior to refactor.

  • Next issue I found is with converting the legacy json file to the new version. I have yet to do any research or triage into this one yet.
    • The back up did work
    • The plugin notified me with success
    • But I found that the new file was empty 😦!
    • Upon next start up I had an error saying that the json file could not be parsed. Perhaps an empty json should have it's own warning instead of not being able to parse.

None of these issues are really that important in my opinion. If I were to make any changes for either of them what would be the best way:

  • Should I create a new issue
  • Just start I pull request
  • Or just plan more here

@kndndrj kndndrj mentioned this issue Oct 17, 2022
@kndndrj
Copy link
Owner

kndndrj commented Oct 17, 2022

Thanks for looking into this!
First off, I'd say that we move this discussion into the opened pull request (#5)

I replied to you there :D

@derekthecool
Copy link
Contributor Author

I see you merged to master! Woo-hoo!

Should we close this ticket or keep it open to keep planning?

@kndndrj
Copy link
Owner

kndndrj commented Oct 28, 2022

@derekthecool I guess we can keep it open for a while, to use it as a sort of mailing list :)

@kndndrj
Copy link
Owner

kndndrj commented Sep 1, 2023

@derekthecool Hey, if you are still interested in this plugin, I revisited it recently - check #9 if you wanna know more.

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

No branches or pull requests

2 participants