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.

Sign up for GitHub

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

Jump to bottom

Suggest correct path in include_bytes! #121833

Merged
merged 2 commits into from
Mar 28, 2024
Merged

Suggest correct path in include_bytes! #121833

merged 2 commits into from
Mar 28, 2024

Conversation

kornelski
Copy link
Contributor

include_bytes! paths are relative, and I'm often not sure how nested is the .rs file that I'm editing, so I have to guess the number of "../..". This change searches .. and ../.. for the given file and offers corrected path as a suggestion.

I wasn't sure how to get the right span, and how to properly escape it.

error: couldn't read src/file.txt: No such file or directory (os error 2)
 --> src/main.rs:2:13
  |
2 |     let x = include_bytes!("file.txt");
  |             ^^^^^^^^^^^^^^^----------^
  |                            |
  |                            help: it's in a parent directory: `"../../file.txt"`

@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2024

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 1, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
Noratrieb
Noratrieb reviewed Mar 1, 2024
View reviewed changes
Copy link
Member

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea, especially the way you suggest making this part of your workflow :)

compiler/rustc_builtin_macros/src/source_util.rs Outdated Show resolved Hide resolved
compiler/rustc_builtin_macros/src/source_util.rs Outdated Show resolved Hide resolved
@kornelski kornelski force-pushed the parent_include branch 6 times, most recently from dee40b1 to 4040f9f Compare March 1, 2024 15:45
@bors
Copy link
Contributor

bors commented Mar 5, 2024

☔ The latest upstream changes (presumably #121998) made this pull request unmergeable. Please resolve the merge conflicts.

estebank
estebank approved these changes Mar 5, 2024
View reviewed changes
compiler/rustc_builtin_macros/src/source_util.rs Outdated Show resolved Hide resolved
compiler/rustc_builtin_macros/src/source_util.rs Outdated Show resolved Hide resolved
compiler/rustc_builtin_macros/src/source_util.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@estebank
Copy link
Contributor

estebank commented Mar 6, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 6, 2024

📌 Commit da8cbe4 has been approved by estebank

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 6, 2024
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 6, 2024
@matthiaskrgr
Rollup merge of rust-lang#121833 - kornelski:parent_include, r=estebank
e3c356a 
Suggest correct path in include_bytes!

`include_bytes!` paths are relative, and I'm often not sure how nested is the `.rs` file that I'm editing, so I have to guess the number of `"../.."`. This change searches `..` and `../..` for the given file and offers corrected path as a suggestion.

I wasn't sure how to get the right span, and how to properly escape it.

```text
error: couldn't read src/file.txt: No such file or directory (os error 2)
 --> src/main.rs:2:13
  |
2 |     let x = include_bytes!("file.txt");
  |             ^^^^^^^^^^^^^^^----------^
  |                            |
  |                            help: it's in a parent directory: `"../../file.txt"`
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2024
@bors
Auto merge of rust-lang#122097 - matthiaskrgr:rollup-musrvu6, r=matth…
cf3248a 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#113518 (bootstrap/libtest: print test name eagerly on failure even with `verbose-tests=false` / `--quiet`)
 - rust-lang#117199 (Change the documented implicit value of `-C instrument-coverage` to `=yes`)
 - rust-lang#121190 (avoid overlapping privacy suggestion for single nested imports)
 - rust-lang#121382 (Rework `untranslatable_diagnostic` lint)
 - rust-lang#121833 (Suggest correct path in include_bytes!)
 - rust-lang#121959 (Removing absolute path in proc-macro)
 - rust-lang#122038 (Fix linting paths with qself in `unused_qualifications`)
 - rust-lang#122051 (cleanup: remove zero-offset GEP)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
#122097 (comment)

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 6, 2024
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 25, 2024
@kornelski
Copy link
Contributor Author

I've made error message normalization normalize the error code too

@workingjubilee
Copy link
Member

@bors r=estebank

@bors
Copy link
Contributor

bors commented Mar 26, 2024

📌 Commit 0e4d790 has been approved by estebank

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 26, 2024
@bors
Auto merge of rust-lang#121833 - kornelski:parent_include, r=estebank
b47a377 
Suggest correct path in include_bytes!

`include_bytes!` paths are relative, and I'm often not sure how nested is the `.rs` file that I'm editing, so I have to guess the number of `"../.."`. This change searches `..` and `../..` for the given file and offers corrected path as a suggestion.

I wasn't sure how to get the right span, and how to properly escape it.

```text
error: couldn't read src/file.txt: No such file or directory (os error 2)
 --> src/main.rs:2:13
  |
2 |     let x = include_bytes!("file.txt");
  |             ^^^^^^^^^^^^^^^----------^
  |                            |
  |                            help: it's in a parent directory: `"../../file.txt"`
```
@bors
Copy link
Contributor

bors commented Mar 26, 2024

⌛ Testing commit 0e4d790 with merge b47a377...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Mar 26, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 26, 2024
@kornelski
Copy link
Contributor Author

I've made it change \ to / in paths to make the paths consistent across platforms.

@workingjubilee
Copy link
Member

hmm, do we usually emit "localized" or "Unixed" paths in diagnostics?

@bors r=estebank

@bors
Copy link
Contributor

bors commented Mar 28, 2024

📌 Commit 826ddb3 has been approved by estebank

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2024
@kornelski
Copy link
Contributor Author

There are other diagnostics that print file paths unmodified, but this one is special, because the suggested path may be written back to the source code.

Paths with forward slashes work on Windows, but paths with backslashes won't work elsewhere.
I assume that Rust prefers portable source code, so / is the better choice.

@workingjubilee
Copy link
Member

ah, that makes sense! was just wondering why the choice.

@bors
Copy link
Contributor

bors commented Mar 28, 2024

⌛ Testing commit 826ddb3 with merge 463a11b...

@bors
Copy link
Contributor

bors commented Mar 28, 2024

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 463a11b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 28, 2024
@bors bors merged commit 463a11b into rust-lang:master Mar 28, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 28, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (463a11b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.5% [-0.6%, -0.5%] 2
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -0.5% [-0.6%, -0.5%] 2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 0.5% [0.4%, 0.5%] 2
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.5% [0.4%, 0.5%] 2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 669.547s -> 671.737s (0.33%)
Artifact size: 315.72 MiB -> 315.68 MiB (-0.01%)

@kornelski kornelski deleted the parent_include branch April 8, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Noratrieb Noratrieb Noratrieb left review comments

@workingjubilee workingjubilee workingjubilee left review comments

@ChrisDenton ChrisDenton ChrisDenton left review comments

@Urgau Urgau Urgau left review comments

@estebank estebank estebank approved these changes

Assignees

@estebank estebank

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.