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

Configuring the Output Directory #39

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

liamrosenfeld
Copy link

Allows saving files to other directories (relative to the workspace root) by setting a VSCode setting.

When started with %SOURCE_DIR%, it refers to the directory containing the current file. That is the default value, preserving the current behavior. That is the cleanest and most extensible method I've thought of so far. If anyone has a better system, I can update this.

%SOURCE_DIR% refers the the directory containing the current file. That is the default value to preserve the current behavior.
Copy link
Owner

@nvarner nvarner left a comment

Choose a reason for hiding this comment

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

This would be a great change, but %SOURCE_DIR% feels like a hack. How about using null instead? See this example in the docs for what that might look like.

@liamrosenfeld
Copy link
Author

liamrosenfeld commented Mar 29, 2023

That is what I initially built, but I switched away from it because a ["string", "null"] setting type does not have a graphical editor in VSCode (it just presents a button to edit it directly in the JSON file).

Another possible method I just thought of would be to have a separate setting that is a dropdown of 'source file directory' or 'workspace directory' to select what the output directory is relative to. That would also keep the functionality of saving in a subdirectory of the source file directory (which %SOURCE_DIR%/subdir allows for). The main downside of this method is that it has two settings to control one thing.

I agree that %SOURCE_DIR% is a hack, so I would be happy to switch to whichever of those you think is better.

@beeb
Copy link
Contributor

beeb commented Mar 29, 2023

Another possible method I just thought of would be to have a separate setting that is a dropdown of 'source file directory' or 'workspace directory' to select what the output directory is relative to.

I also think this would be preferrable. One setting called something like "output directory root" (dropdown, with maybe a third option being "absolute path", and the default being "source file directory") and another "output directory path" relative to the root selected above.

removes the `%SOURCE_DIR%` workaround
@liamrosenfeld
Copy link
Author

Just pushed the setting split

@beeb
Copy link
Contributor

beeb commented Mar 29, 2023

Should an absolute path be an option? Trying to think if people would actually want to output to a directory completely unrelated to their workspace or source dir.

@liamrosenfeld
Copy link
Author

liamrosenfeld commented Mar 29, 2023

I think that could be useful. They might want to have a single cache folder for all of their live previews to intermittently clear. I'll implement it.

@liamrosenfeld
Copy link
Author

I implemented the absolute option. I just need to fix the issues caused by the merge conflicts.

@liamrosenfeld
Copy link
Author

I resolved the merge conflict issues

@beeb
Copy link
Contributor

beeb commented Mar 30, 2023

Sorry to comment again on this... Should people be able to customize the name of the output file? Should it be then included in the path setting directly or be a separate setting? This could also be added as a later date, or not at all.

"enumDescriptions": [
"The folder containing the source file",
"The VSCode workspace",
"Your home directory"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be "The root of the filesystem"?

Copy link
Author

Choose a reason for hiding this comment

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

That would make sense. I was designing it with a Unix filesystem in mind where VSCode would only have permission to save into the home directory anyway. I guess that is misleading when calling it "Absolute". I can change it to absolute and ensure it can handle ~/.

src/main.rs Outdated
file.to_file_path().unwrap().parent().unwrap().to_path_buf()
}
config::OutputRoot::Workspace => world.root().to_path_buf(),
config::OutputRoot::Absolute => home::home_dir().unwrap(),
Copy link
Contributor

@beeb beeb Mar 30, 2023

Choose a reason for hiding this comment

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

I would make the root path an Optional, and make it None for Absolute. When the root is OutputRoot::Absolute, we should use the outputPath only, which should include / or C:\ (be an absolute path), and ignore the root.

@nvarner
Copy link
Owner

nvarner commented Mar 30, 2023

The ["string", "null"] editor is unfortunate, since that would a nice solution. To propose another alternative, we could keep one string for the output directory and treat it like a CLI tool would.

By default, it would be relative to the workspace root (or some other project-specific directory), so output/pdf/ would target a subdirectory of the workspace. Then, if the user enters an absolute path, like /home/username/typst_goes_here, output files to that directory unrelated to the workspace.

@nvarner
Copy link
Owner

nvarner commented Mar 30, 2023

I don't like leaving in hacks, but I'm actually okay with small ones here. As Typst matures, I expect its offline users will need to manage growing projects, collaborators over Git repositories will need to share local configurations, the planned package manager will need a way to provide packages, and so on. We'll probably see some kind of build system tool, like cargo, that saves its configuration into a file that we can read. That should specify where to output files.

Basically, given enough time, I think this feature will be made redundant, so we can invest less in future-proofing it.

@liamrosenfeld
Copy link
Author

Sorry to comment again on this... Should people be able to customize the name of the output file? Should it be then included in the path setting directly or be a separate setting? This could also be added as a later date, or not at all.

I think that changing the name would be best suited for another PR. These changes are extensible to support it in the future, but that brings up a new collection of setting design questions that would probably be suited for another PR.

To propose another alternative, we could keep one string for the output directory and treat it like a CLI tool would.

The primary problem with this solution is it would be unable to keep the existing behavior of saving to the source file directory.

@liamrosenfeld
Copy link
Author

I made the absolute path actually absolute (with tilde expansion)

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.

None yet

3 participants