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

un-nest imports.lock, treat it as N SourceFiles #238

Closed
Gankra opened this issue Jun 29, 2022 · 4 comments
Closed

un-nest imports.lock, treat it as N SourceFiles #238

Gankra opened this issue Jun 29, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request p1 do now

Comments

@Gankra
Copy link
Contributor

Gankra commented Jun 29, 2022

Right now we build a mega-toml of all your imports, but this can significantly change the imported file. We should instead have imports.lock be a container format for N separate toml files, largely verbatim.

The format should be

import_name1:num_lines1
<a num_lines1 toml file>
import_name2:num_lines2
<a num_lines2 toml file>

By having the container format specify the length of its contents, we don't need any kind of escaping and can preserve the file pretty literally. This should give better diffs on updates. That said we will still be formatting it implicitly, and in #237 I am establishing that we will set criteria.description if the original input only had criteria.url.

When loading the lockfile we should also logically identify the subsections as their own SourceFiles for precise error-reporting.

@Gankra Gankra added enhancement New feature or request p1 do now labels Jun 29, 2022
@mystor
Copy link
Collaborator

mystor commented Jun 29, 2022

Would it make sense for us to consider having imports.lock be a directory and having one file per import? That would certainly make things a bit simpler to implement.

@bholley
Copy link
Collaborator

bholley commented Jun 29, 2022

Would it make sense for us to consider having imports.lock be a directory and having one file per import? That would certainly make things a bit simpler to implement.

I think that would lead to errors of people forgetting to git add the file. So I think the extra tooling overhead of a megafile is worth it.

@mystor
Copy link
Collaborator

mystor commented Jun 29, 2022

One other option from the suggestion from comment 1 is to use a TOML file with multiline string literals for each embedded file. This would definitely lead to some nested escaping in some cases, but would be very simple for us to implement and could preserve the source file byte-for-byte.

@mystor
Copy link
Collaborator

mystor commented Jul 20, 2022

Based on discussions in #272 and on matrix it looks like we're probably not taking this approach, so closing for now.

@mystor mystor closed this as completed Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p1 do now
Projects
None yet
Development

No branches or pull requests

3 participants