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

Use relative paths as file-relative when parsing a file #6150

Merged
merged 3 commits into from Jul 27, 2022

Conversation

kubouch
Copy link
Contributor

@kubouch kubouch commented Jul 27, 2022

Description

Currently affects:

  • source
  • use
  • overlay add

If one of the above is used inside a script with a relative filename as an argument, the path is assumed to be relative to the parsed file, not cwd.

Also applies to scripts parsed with nu-check. However, nu-check used inside a script is not affected as we would need to track the currently parsed file within the engine.

Fixes #5853

Related: #4842

Tests

Make sure you've done the following:

  • Add tests that cover your changes, either in the command examples, the crate/tests folder, or in the /tests folder.
  • Try to think about corner cases and various ways how your changes could break. Cover them with tests.
  • If adding tests is not possible, please document in the PR body a minimal example with steps on how to reproduce so one can verify your change works.

Make sure you've run and fixed any issues with these commands:

  • cargo fmt --all -- --check to check standard code formatting (cargo fmt --all applies these changes)
  • cargo clippy --workspace --features=extra -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
  • cargo test --workspace --features=extra to check that all the tests pass

@kubouch kubouch changed the title When parsing a file, use relative paths are path-relative When parsing a file, use relative paths as file-relative Jul 27, 2022
@kubouch kubouch marked this pull request as ready for review July 27, 2022 14:03
@hustcer
Copy link
Contributor

hustcer commented Jul 27, 2022

Great, I've looking forward to this feature for a long time

@kubouch kubouch changed the title When parsing a file, use relative paths as file-relative Use relative paths as file-relative when parsing a file Jul 27, 2022
@kubouch kubouch merged commit c92211c into nushell:main Jul 27, 2022
@jmgilman
Copy link

jmgilman commented Nov 14, 2022

I'm struggling to try to understand what the change was here. Are relative paths relative to the current working directory or are they relative to the location of the file being executed?

On the latest version (0.71.0), if I have the following:

/tmp/test/mod.nu

export def hello [name: string] {
    $"hello ($name)!"
}

/tmp/test/test.nu

use mod.nu

mod hello "world"

And I attempt to run the script:

❯ nu /tmp/test.nu   
Error: nu::parser::module_not_found (link)

  × Module not found.
   ╭─[/tmp/test.nu:1:1]
 1 │ use mod.nu
   ·     ───┬──
   ·        ╰── module not found
 2 │ 
   ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything is evaluated

It doesn't find the module. Looking at the tests introduced by this PR, it's using nu-check, which ironically passes in this case:

❯ nu-check /tmp/test.nu
true

This seems like a bug, but I could be misunderstanding the desired effect here.

@WindSoilder
Copy link
Collaborator

@jmgilman Hi, I can't reproduce it on latest main, would you please verify it again?

@kubouch kubouch deleted the file-relative-parsing branch January 20, 2024 09:48
fdncred pushed a commit that referenced this pull request Apr 9, 2024
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

Fixes #7849, #11465 based on @kubouch's suggestion in
#11465 (comment).

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

Can source files relative to `env.nu` or `config.nu` like in #6150.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

Adds test that previously failed.

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
FilipAndersson245 pushed a commit to FilipAndersson245/nushell that referenced this pull request Apr 10, 2024
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

Fixes nushell#7849, nushell#11465 based on @kubouch's suggestion in
nushell#11465 (comment).

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

Can source files relative to `env.nu` or `config.nu` like in nushell#6150.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

Adds test that previously failed.

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
NotTheDr01ds pushed a commit to NotTheDr01ds/nushell that referenced this pull request Apr 12, 2024
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

Fixes nushell#7849, nushell#11465 based on @kubouch's suggestion in
nushell#11465 (comment).

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

Can source files relative to `env.nu` or `config.nu` like in nushell#6150.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

Adds test that previously failed.

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
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.

source usability
4 participants