vlang / v-analyzer

The @vlang language server, for all your editing needs like go-to-definition, code completion, type hints, and more.
MIT License
86 stars 9 forks source link

chore: remove useless `compiler_flag` and copy `.exe` on windows #108

Closed XiaoPangxie732 closed 1 month ago

XiaoPangxie732 commented 1 month ago

v install should not be concerned with compilation flags. And the actual compiler is determined by build.vsh:

``` PS C:\Users\***\.config\v-analyzer> $Env:CC = 'clang' PS C:\Users\***\.config\v-analyzer> v .\install.vsh up --nightly Installing latest nightly version... Updating v-analyzer sources... ✓ Successfully updated v-analyzer sources Building v-analyzer... ✓ Dependencies for v-analyzer installed successfully Building v-analyzer at commit: 7e11a6f, build time: 2024-06-15 01:17:19 ... ✓ Prepared output directory Building v-analyzer in debug mode, using: "E:\Programs\v\v.exe" "C:\Users\***\.config\v-analyzer\sources" -o "./bin/v-analyzer.exe" -no-parallel -cc clang -g To build in release mode, run v build.vsh release Release mode is recommended for production use. At runtime, it is about 30-40% faster than debug mode. ✓ Successfully built v-analyzer! Binary is located at C:\Users\***\.config\v-analyzer\sources\bin\v-analyzer.exe Moving v-analyzer binary to the standard location... Failed to copy v-analyzer binary to C:\Users\***\.config\v-analyzer\bin: Failed to remove "C:\Users\***\.config\v-analyzer\bin\v-analyzer.exe": Permission denied; code: 13 ✓ v-analyzer successfully updated to nightly (7e11a6f8f369df935664fadd2f0c99d90fe3226f) Path to the binary: C:\Users\***\.config\v-analyzer\bin\v-analyzer ```

Also, the executable has .exe extension on Windows, thus v-analyzer.exe should be copied on Windows

spytheman commented 1 month ago

The analyzer test failure is unrelated, and due to a change in string.is_upper() that happened in https://github.com/vlang/v/pull/21358 , while v-analyzer's CI was not ran after that (we only test that v-analyzer itself builds on the main V CI, and not that v-analyzer's CI passes).

spytheman commented 1 month ago

I am not sure whether to fix pascal_case_to_snake_case or its tests, but I am more inclined to change pascal_case_to_snake_case to preserve its old behavior. @ttytm what do you think?

ttytm commented 1 month ago

https://github.com/vlang/v-analyzer/blob/7e11a6f8f369df935664fadd2f0c99d90fe3226f/src/utils/text_utils.v#L6

- if c.ascii_str().is_upper() {
+ if c.ascii_str().is_upper() || c.is_digit() {
ttytm commented 1 month ago

Would make it pass with the current utils tests. Additionally including a check of the last letter would make it more safe.

spytheman commented 1 month ago

https://github.com/vlang/v-analyzer/blob/7e11a6f8f369df935664fadd2f0c99d90fe3226f/src/utils/text_utils.v#L6

- if c.ascii_str().is_upper() {
+ if c.ascii_str().is_upper() || c.is_digit() {

Applied in 2584e86 .

spytheman commented 1 month ago

(rebased over current main)