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

fix: use str length for str instead of length #596

Closed
wants to merge 2 commits into from

Conversation

mrdgo
Copy link

@mrdgo mrdgo commented Jul 26, 2023

nushell won't start as long as you use length.

[1 2 3 4] | length

works.

"hello world" | str length

works.

Different permutations don't.

@mati865
Copy link

mati865 commented Jul 26, 2023

This won't work, I have tried the same but it fails when trying to use zoxide:

❯ z target
Error: nu::shell::type_mismatch

  × Type mismatch during operation.
    ╭─[/home/mateusz/.zoxide.nu:27:1]
 27 │   let arg0 = ($rest | append '~').0
 28 │   let path = if (($rest | str length) <= 1) and ($arg0 == '-' or ($arg0 | path expand | path type) == dir) {
    ·                  ──────────┬───────── ─┬ ┬
    ·                            │           │ ╰── int
    ·                            │           ╰── type mismatch for operator
    ·                            ╰── list<int>
 29 │     $arg0
    ╰────

@maxim-uvarov
Copy link

@mati865 I believe this is a bug of nushell 0.83.0 (check it here nushell/nushell#9809)

Otherwise it should work.

@fdncred
Copy link

fdncred commented Jul 26, 2023

A hack for this, until we fix the bug, to get people up and running is changing

def-env __zoxide_z [...rest:string]

to

def-env __zoxide_z [...rest]

and do the same for __zoxide_zi

@mati865
Copy link

mati865 commented Jul 26, 2023

@mati865 I believe this is a bug of nushell 0.83.0 (check it here nushell/nushell#9809)

Otherwise it should work.

Yup, that's why I have mentioned it in referenced item.

@mrdgo
Copy link
Author

mrdgo commented Jul 26, 2023

yes I agree, I faced the same issue :) Seemed to work at first

@mrdgo
Copy link
Author

mrdgo commented Jul 26, 2023

updated fix to temporary fix.

@fdncred
Copy link

fdncred commented Jul 26, 2023

fwiw, I'd run with the "temporary fix" for now. I'm not sure how long it'll be to fix it the "right" way. We can do another PR here when ...rest: string works again.

@asim215
Copy link

asim215 commented Jul 26, 2023

I fix it by another way.
See #595 .

@amtoine
Copy link

amtoine commented Jul 27, 2023

hello there from Nushell 👋 😋

so yeah str length is not a drop in replacement for length in that case, because $rest is not a string but a list<string>, which can be seen with a describe as in nushell/nushell#9720 (comment) 👍

this was a bug with how $rest was passed around and has been fixed upstream in nushell/nushell#9816 😌
should be available soon in a patch release 😊

@ajeetdsouza
Copy link
Owner

That's good news! In that case, I think it would be better to wait for the patch release rather than release a hack here.

@amtoine
Copy link

amtoine commented Jul 28, 2023

That's good news! In that case, I think it would be better to wait for the patch release rather than release a hack here.

we're working on it... ♻️
i'd say the best and safest is to keep those kind of PRs opened and ready, just in case, and all stay in touch. how's that sound? 😇

@ajeetdsouza
Copy link
Owner

This seems to be fixed in Nushell, so I'm closing this. Thanks!

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.

7 participants