Refactor

[kndndrj]

Whole plugin written from scratch.

For general overview see the new README.md

[kndndrj]

Reply to #2 (comment)

Hi @derekthecool. I am really glad that you like the refactored version!!

As for the issues:

• I'm up for starting the wiki, but maybe let's make the new version stable first - there may be some small breaking changes to be done (im not sure)
• cwd behaviour should be the same as it was before (like you noticed, its just using vim.fn.getcwd()), so I am really surprised that the behaviour has changed for you 🤔. It's basically just the directory from where you start vim - or you can change it by :cd .... One possible solution for this would be to maybe detect git directories and look for files there - but on the other hand, that may be a job of the end user, to set the parameter to the loader. So a setup function like this may be a good entry to our wiki. - what do you think? :)
• I'm a bit frustrated that legacy json converting has failed so miserably tbh 😄. If you'd like to fix this, let me know... but I don't think its necessary to waste a lot of time on this feature, since these legacy loaders should be deprecated pretty soon. (If you want, you can send me your old projector.json, so I can test a bit (remove any sensitive info before :)) - Using windows could be the issue here, since I didn't have a chance to test it on windows :(.

I'd ask you not to rush too much with making the changes, because, as I said, there are some stuff that I'd like to change a bit (namely loader and output options - I think there should be more parameters to add to the loaders, and outputs should be customizable too).

I'd be happy to hear someone else's opionion on this!

[derekthecool]

@kndndrj,

I'm getting good feelings about this plugin still, I really think this tool helps so much!

• wiki -- good!
• Yes, let's leave that alone for now and once I find more details a wiki item will be perfect
• I tested the json convert on Linux and windows. I primarily use Linux but need windows for work so I am testing both. I have no strong feelings about keeping the legacy feature. I say we just abandon it. If there where thousands of user of this plugin we'd probably want to be more careful. But since we don't, we can chop out whatever feels right.

Testing today I did find a non cross platform item.

nvim-projector/lua/projector/outputs/builtin.lua

Line 44 in 175e40f

vim.fn.termopen("sh -c '" .. command .. "'", term_options)

Perhaps we could change to this instead? it'll use your default shell, but should work well for all platforms. I've proved this works on windows, but have yet to check for regressions on Linux.

vim.fn.termopen(command, term_options)

[kndndrj]

@derekthecool, I agree regarding legacy converter. As for the sh -c... thingy - without that, pipes from command won't work. maybe we can detect a default shell and then run it with that. Would really appretiate if you could do some testing on windows with powershell or cmd. I'm not sure how we'd fix this... open to suggestions

[derekthecool]

Regarded shell command runner, I've done some more testing.

Continuing to use the option I suggested seems to work very well on windows and Linux. I think this plugin would be best off keeping the user terminal settings such as vim.opt.shell, vim.opt.shellcmdflag etc. Auto detection is not too complicated but I think it is not necessary.

vim.fn.termopen(command, term_options)

Let me show you some of my examples using piped commands to see if I understand you correctly. Let me know if I have not understood you correctly with the use of pipes.

Linux example using zsh shell

json

[
  {
    "name":  "testing shell pipes",
    "group": "shell",
    "command": "echo $SHELL; ls -la | tail -n3"
  }
]

output

/bin/zsh
drwxr-xr-x  4 derek derek  4096 Apr 29  2021 release
drwxr-xr-x  3 derek derek  4096 Jul 20 15:33 test
-rwxr-xr-x  1 derek derek  3233 Jun 23 15:01 updateFromFtp.sh

[Process exited 0]

windows example using PowerShell

[
  {
    "name":  "testing shell pipes",
    "group": "shell",
    "command": "ls | select-object -last 2"
  }
]

    Directory: C:\Users\Derek Lomax\source\repos\DatabaseTesting     

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a---          10/16/2022  5:32 PM           2192 DatabaseTesting.sln
-a---          10/18/2022  8:37 AM           1226 projector.json     


[Process exited 0]

I found that use the similar method to sh -c ...
with PowerShell using
pwsh -c ...

  vim.fn.termopen("pwsh -c '" .. command .. "'", term_options)

Fails to respect my vim option vim.opt.shellcmdflag. And I get tons of warnings because it is tying to load my PowerShell profile which I don't want.

[derekthecool]

I never addressed the args parameter in my last message.

More tweaking could be done there later. For now I'm just trying to prove that the core command and not specifying any shell options will work.
If you agree that it works for you with pipes and we can look for improvements to using the args from config better.

[derekthecool]

Brief update. I've been using my own edited version of outputs/builtin.lua with this code

vim.fn.termopen(command, term_options)

I've had a wonderful day using this plugin all day at work. Using the task runner and the dependency task runner has been amazing! Being able to build my project and launch right into debugger, wow wow wow! That is powerful stuff.

Some additional questions

• I've had an idea that it would be really cool to be able to attach a key mapping a task. What do you think?
• Also, I wanted to start adding some plenary tests soon. Do you think we should wait until after merging refactor to master?
• Speaking of merging to master, when do you think is the right time to do that?

[kndndrj]

Well, I have no Idea what I was doing before... I tested it now and pipes really DO work 😄. I was probably missing some stuff with "args" and it didn't work because of that. Anyways, I removed the unnecessary wrapper.

As for merging... I'd let it sit for a couple more days. let's say that we merge it on Friday afternoon (speaking from yurop perspective)

[derekthecool]

Haha, no worries. I am glad it works for you too.

Okay that sounds like a good plan. Very exciting.
I have not ran into any issues today btw.