Skip to content

Conversation

gdotdesign
Copy link
Member

Currently, assets an only be referenced relative to the file which references them. As per the issue, if there is a file deep in the source directory, referencing a file in the project root requires a lot of ../.

The PR makes it so that the root path / is the project root directory which have the mint.json file.

Resolves #459

@gdotdesign gdotdesign added the enhancement New feature or request label Nov 18, 2024
@gdotdesign gdotdesign added this to the 0.21.0 milestone Nov 18, 2024
@gdotdesign gdotdesign requested a review from Sija November 18, 2024 16:12
@gdotdesign gdotdesign self-assigned this Nov 18, 2024
@@ -11,7 +11,7 @@ Dir
sample, expected = File.read(file).split("-" * 80)

# Parse the sample
ast = Mint::Parser.parse(sample, file)
ast = Mint::Parser.parse(sample, File.dirname(__FILE__) + file.lchop("./spec"))
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of chopping off the ./spec from the filepath?

Copy link
Member Author

Choose a reason for hiding this comment

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

File.dirname(__FILE__) is the spec directory, so we need to remove that from the test file.

Copy link
Member

Choose a reason for hiding this comment

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

And what's wrong with that?

Copy link
Member Author

Choose a reason for hiding this comment

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

It would not find the assets defined in specs.

/path/to/spec/.spec/test-file where /path/to/spec/.spec is not a file or a directory.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't because you've added a mint.json file to the spec/ folder?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the spec folder becomes the root folder / for the tests. So for example /fixtures/data.txt is reachable so as ../../fixtures/data.txt from spec/compilers/directives/inline test.

Copy link
Member

Choose a reason for hiding this comment

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

And why don't you reference the files from the root folder instead? Unless I'm mistaken this way it wouldn't need any hacks.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't follow. We would still need to chop ./spec off. It doesn't matter if it's absolute or relative. And we still need to find the closest mint.json to figure out the root folder.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps I don't fully grok the situation. I'm gonna approve it and come back to it later, once I familiarize myself with the recent changes - and there's a lot of 'em 😅

Co-authored-by: Sijawusz Pur Rahnama <sija@sija.pl>
@gdotdesign gdotdesign requested a review from Sija November 20, 2024 08:20
@gdotdesign gdotdesign merged commit ada378a into master Nov 21, 2024
3 checks passed
@gdotdesign gdotdesign deleted the absolute-asset-path branch November 21, 2024 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add asset path alias
2 participants