crystal-lang-tools / vscode-crystal-lang

Yet another VSCode extension for Crystal Programming Language
https://marketplace.visualstudio.com/items?itemName=crystal-lang-tools.crystal-lang
MIT License
274 stars 56 forks source link

New release? #174

Closed jwoertink closed 5 months ago

jwoertink commented 11 months ago

The last release 0.8.4 was added on 2022-1-27, 11:51:14. Since that release was tagged, there's been several updates made (18 commits), including a few (https://github.com/crystal-lang-tools/vscode-crystal-lang/pull/165, https://github.com/crystal-lang-tools/vscode-crystal-lang/pull/173) PRs that may be ready to go.

I've never released a plugin for VSCode, so I'm not sure how involved it is, but I would like to at least get the conversation started on what all it might take to get a new release out. Or maybe a beta release to test out all of these changes?

Thank you!

nobodywasishere commented 11 months ago

I've been working on testing the current code to make sure everything works across the various platforms before release (linux, windows, macos) and to debug issues people have ran into. I've conversed with @bcardiff about this and we should be able to do a release when everything's ready, particularly when #173 is merged.

jwoertink commented 11 months ago

Awesome! That's great to hear :rocket: Thanks for the update :heart:

nobodywasishere commented 11 months ago

Thank you as well for reaching out! I'll use this issue for keeping track of the upcoming release.

nobodywasishere commented 11 months ago

As part of helping me test this for release, I put together a list of features, short descriptions, how they work at a high level, and known current issues. Eventually I want to work this into the wiki, but for now it's a good starting point for what to test to ensure everything works. If anyone is up to help assist in testing this before release, I've attached a development version of the current changes on #173.

crystal-lang-0.8.5.vsix.zip

Features

Platform Support Goal (at minimum)

jwoertink commented 11 months ago

nice! I'm down to test this. How do I install that zip? Unpack it, and move it to some vscode directory? Looks like I have a ~/.vscode/extensions/ directory... Great work on this!

nobodywasishere commented 11 months ago

Thank you! To install it, you just need to extract it and install it from within vscode: https://code.visualstudio.com/docs/editor/extension-marketplace#_install-from-a-vsix

Make sure to reload the window after installing to ensure you're running the right version.

jwoertink commented 11 months ago

Ok, seeing a few things here... Would you like me to list them out in this thread? or just open up new issues?

nobodywasishere commented 11 months ago

We can start with a comment here with a basic overview of the issues you're seeing, including OS information and stuff. From there we can create issues.

jwoertink commented 11 months ago

When I save one of my models, sometimes it'll randomly say that the class it inherits from doesn't exist: image If I open up that BaseModel file, then go back to my Emote model, the error goes away. Seems it also happens with included modules image image

❯ neofetch
             /////////////                jeremy@pop-os 
         /////////////////////            ------------- 
      ///////*767////////////////         OS: Pop!_OS 22.04 LTS x86_64 
    //////7676767676*//////////////       Host: Thelio Mira thelio-mira-r1 
   /////76767//7676767//////////////      Kernel: 6.4.6-76060406-generic 
  /////767676///*76767///////////////     Uptime: 3 days, 1 hour, 10 mins 
 ///////767676///76767.///7676*///////    Packages: 2912 (dpkg), 56 (flatpak), 20 (snap) 
/////////767676//76767///767676////////   Shell: bash 5.1.16 
//////////76767676767////76767/////////   Resolution: 1920x1080, 3840x2160 
///////////76767676//////7676//////////   DE: GNOME 42.5 
////////////,7676,///////767///////////   WM: Mutter 
/////////////*7676///////76////////////   WM Theme: Pop 
///////////////7676////////////////////   Theme: Pop [GTK2/3] 
 ///////////////7676///767////////////    Icons: pop-os-branding [GTK2/3] 
  //////////////////////'////////////     Terminal: gnome-terminal 
   //////.7676767676767676767,//////      CPU: AMD Ryzen 5 5600X (12) @ 3.700GHz 
    /////767676767676767676767/////       GPU: NVIDIA GeForce RTX 3070 Ti 
      ///////////////////////////         Memory: 10001MiB / 64202MiB 
         /////////////////////
             /////////////                                        
Version: 1.81.0
Commit: 6445d93c81ebe42c4cbd7a60712e0b17d9463e97
Date: 2023-08-02T12:36:11.334Z
Electron: 22.3.18
ElectronBuildId: 22689846
Chromium: 108.0.5359.215
Node.js: 16.17.1
V8: 10.8.168.25-electron.0
OS: Linux x64 6.4.6-76060406-generic snap
nobodywasishere commented 11 months ago

Do you have 'mainFile' set in your settings for this workspace? It's unfortunately a requirement for a lot of functionality provided by 'crystal tool', and I do think we should either pull it from 'targets' in shards.yml or make it more prominent it needs to be set.

jwoertink commented 11 months ago

I don't have that field set.
image

Since the main file changes per project, what would be the best way to set this? For example, on each Lucky project, the main file is the name of the project, so if my project is named foo, then my main file would be src/foo.cr. I could use src/start_server.cr, but then when I'm not in a Lucky project, that wouldn't exist.

Thinking about it though, it would make things easier if we adopted a single src/main.cr for every project always :joy: Though, crystal init app test generates a src/test.cr file for the main :thinking:

jwoertink commented 11 months ago

Ok, adding that main does seem to solve that issue :+1:

nobodywasishere commented 11 months ago

Maybe we can default to src/project_name.cr if mainFile isn't set? Also you can have vscode settings be per project afaik, and it'll save it in a '.vscode/settings.json' in the project itself, so you don't need to set it globally for all projects. The documentation/error handling for this could be better.

jwoertink commented 11 months ago

Ok, I think I have one more thing... I've tanked my machine twice today by fulling running out of (64GB) memory...

When I type a lot of comments, the system seems to just come to a crawl then crash. As in, I'm just commenting on code, and then it all goes bonkers

image

nobodywasishere commented 11 months ago

Oh god I'm sorry! I have no idea what could've caused that, maybe too many instances of the compiler running? Honestly, I'm almost to the point of leaving the current version as-is and moving over to put my efforts into the rewrite. This was really only supposed to be small fixes to make things easier and be a hold-over until the rewrite was stable but now I'm unsure if it's worth it.

jwoertink commented 11 months ago

maybe too many instances of the compiler running

Yeah, I'm pretty sure this is what it was. I was able to run top after and saw like 30 crystal compilers that seemed to be doing stuff. Once VSCode was closed, and I waited a few minutes, everything returned to normal. Maybe it was just trying to run a new one on each keystroke or something? We actually had a similar issue in Lucky a while back. There's a built-in watcher and each time you'd save a file, it kicks off a new crystal compiler to recompile the app. The issue was if you made several changes and saved several files, it would kick off a new one for each file saved.

To fix that, we had to basically store the current process, and if a new save comes in, then we just stop that one and do a new one.

https://github.com/luckyframework/lucky/blob/d60191eb057230f8eb2cd4d446ad791742284613/tasks/watch.cr#L159-L198

bcardiff commented 9 months ago

what would be the best way to set this?

Using the shards.yml targets values if they exists seems like a good default. But I am not sure how to deal when there are multiple.

Having an option to use specs as main file (that will require **/*_spec.cr on the fly) might be a good checkbox/option to have since that is neither a target in shards.tml nor is usually the case of a single file that includes all specs.

nobodywasishere commented 9 months ago

Just added a commit that treats specs as their own main files, ignoring the mainFile if set, when executing implementations/etc. I also reverted enabling completions / implementations / hover as I believe one of them was causing the memory issue and I haven't had the time to investigate which.

y8 commented 8 months ago

Thanks for the build!

I'm having issues with build task for a project where shards.yml have more than one target.

For example, I have a list of targets:

targets:
  agent:
    main: src/app/agent.cr
  beacon:
    main: src/app/beacon.cr
  server:
    main: src/app/server.cr    

And build task:

{
  "version": "2.0.0",
  "tasks": [
    {
      "type": "crystal",
      "command": "run",
      "file": "src/app/beacon.cr",
      "label": "Crystal: run - src/app/beacon.cr",
      "group": {
        "kind": "build",
        "isDefault": true
      }
    }
  ]
}

When I'm trying to launch this task I'm getting:

Error: The crystal task detection didn't contribute a task for the following configuration:
{
    "type": "crystal",
    "command": "run",
    "file": "src/app/beacon.cr",
    "label": "Crystal: run - src/app/beacon.cr",
    "group": {
        "kind": "build",
        "isDefault": true
    }
}
The task will be ignored.

It seems like task accept only first target, failing to launch any others.

Also it doesn't work with arbitrary files, for example this task that that supposed to run currently open file won't work either:

{
  "version": "2.0.0",
  "tasks": [
    {
      "type": "crystal",
      "command": "run",
      "file": "${file}",
      "problemMatcher": [],
      "label": "Crystal: run current file",
      "group": {
        "kind": "build",
        "isDefault": true
      }
    }
  ]
}

Maybe I'm missing something in configuration?

nobodywasishere commented 7 months ago

@y8 thank you for the writeup, I'm going to create a separate issue for this