Skip to content

Attempt at fixing get command panic.#777

Merged
andrasio merged 2 commits intonushell:masterfrom
JonnyWalker81:fix-get-panic
Oct 3, 2019
Merged

Attempt at fixing get command panic.#777
andrasio merged 2 commits intonushell:masterfrom
JonnyWalker81:fix-get-panic

Conversation

@JonnyWalker81
Copy link
Copy Markdown
Contributor

@JonnyWalker81 JonnyWalker81 commented Oct 3, 2019

If possible matches are not found then check if the passed in obj parameter is a string or a path, if so then return it. I am not sure this is the right fix, but I figured I would make an attempt and get a conversation started about it. This PR is to address #707.

If possible matches are not found then check if the passed in `obj`
parameter is a `string` or a `path`, if so then return it.  I am not
sure this is the right fix, but I figured I would make an attempt and
get a conversation started about it.
@sophiajt
Copy link
Copy Markdown
Contributor

sophiajt commented Oct 3, 2019

You may also want to write an error out in the case there are no possible matches, but with a simpler error (on phone but maybe you could use a labeled_error that just says the column is unknown)

@JonnyWalker81
Copy link
Copy Markdown
Contributor Author

Do you mean write an error and continue to the last match block to check for a string or a path? If I return an error (in an else case when checling possible_matches it would return from the function and not go through the last match block). Maybe I am not fully understanding your comment?

@sophiajt
Copy link
Copy Markdown
Contributor

sophiajt commented Oct 3, 2019

@JonnyWalker81 before the PR, it looked like we always returned an error of the column name didn't match. I was wondering if we should keep that, or is that part of the problem?

@JonnyWalker81
Copy link
Copy Markdown
Contributor Author

The reason for the panic was from the possible_matches was empty but the error was indexing into it. I added a length check to prevent the panic and added a match at the end to check for more specific Value types, if it matches then return that. So if there are possible_matches then it would still return an error like before.

@andrasio
Copy link
Copy Markdown
Member

andrasio commented Oct 3, 2019

@JonnyWalker81 Looks good!

@andrasio andrasio merged commit 3c6ee63 into nushell:master Oct 3, 2019
elferherrera pushed a commit to elferherrera/nushell that referenced this pull request Feb 7, 2022
kubouch pushed a commit that referenced this pull request Feb 7, 2022
bobhy added a commit to bobhy/nushell that referenced this pull request Oct 22, 2023
* Release notes for `0.76`

Please add your important new features and breaking changes to the release notes by committing to/opening a PR against the `release-notes-0.76` branch. Thank you!

* Add breaking change for plugin signature (nushell#775)

* add breaking change

* Update blog/2023-02-21-nushell_0_76.md

---------

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>

* add some info on debugging commands

* release notes for nushell#7952 (nushell#777)

* release notes for nushell#7952

* Fix html tags that broke CI

* more debug notes

* Add `profile` note and screenshot (nushell#778)

* add ast to debug commands section

* add breaking change (nushell#790)

* Remove example stuff

Don't let the lorem ipsum loose

* added more breaking changes notes

* trim down error message documentation in blog post

* Add description of some commands

* Do some polishing. sequence multiplication

* Screenshot help of a plugin

* Add section on nu plugin

* Add section on background work and full log

* Executive summary

* Details to "mul"

---------

Co-authored-by: WindSoilder <WindSoilder@outlook.com>
Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
Co-authored-by: Bob Hyman <bob.hyman@gmail.com>
Co-authored-by: Jakub Žádník <kubouch@gmail.com>
Co-authored-by: Reilly Wood <reilly.wood@icloud.com>
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.

3 participants