Skip to content
This repository has been archived by the owner on Jun 8, 2024. It is now read-only.

Added support for dot config (.typstfmt.toml) #109

Merged
merged 5 commits into from
Nov 4, 2023
Merged

Added support for dot config (.typstfmt.toml) #109

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4e83a99
Added support for dot config (.typstfmt.toml)
Andrew15-5 Oct 2, 2023
a4b82ee
chore(config name): removed `DOT_CONFIG_FILE_NAME`
Andrew15-5 Oct 29, 2023
449c484
feat(config file): warn when multiple configs found
Andrew15-5 Oct 29, 2023
e2af19b
Merge branch 'master' into dot-config-support
Andrew15-5 Oct 31, 2023
8e6081f
feat(UX): included config name into error messages
Andrew15-5 Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use typstfmt_lib::{format, Config};

const VERSION: &str = env!("TYPSTFMT_VERSION");
const CONFIG_FILE_NAME: &str = "typstfmt.toml";
const DOT_CONFIG_FILE_NAME: &str = ".typstfmt.toml";
const HELP: &str = r#"Format Typst code

usage: typstfmt [options] [file...]
Expand Down Expand Up @@ -191,7 +192,8 @@ fn main() -> Result<(), lexopt::Error> {
}

let config = {
if let Ok(mut f) = File::options().read(true).open(CONFIG_FILE_NAME) {
let open_config = |file_name| File::options().read(true).open(file_name);
if let Ok(mut f) = open_config(CONFIG_FILE_NAME).or(open_config(DOT_CONFIG_FILE_NAME)) {
Andrew15-5 marked this conversation as resolved.
Show resolved Hide resolved
let mut buf = String::default();
f.read_to_string(&mut buf).unwrap_or_else(|err| {
panic!("Failed to read config file {CONFIG_FILE_NAME:?}: {err}")
Copy link
Owner

Choose a reason for hiding this comment

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

I feel like now, this can be confusing, can I ask you to fix it by replacing this line
panic!("Failed to read config file {CONFIG_FILE_NAME:?}: {err}")
with
panic!("Failed to read config file {CONFIG_FILE_NAME:?} (or/then .{CONFIG_FILE_NAME:?}): {err}")
Or something else if you have a better idea?

If you're tired of this PR and want to merge already no problem, I can take care of it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about next error:

Config::from_toml(&buf).unwrap_or_else(|e| panic!("Config file invalid: {e}.\nYou'll maybe have to delete it and use -C to create a default config file."))

Error about reading config file should include file name, but TOML parser error shouldn't?

Copy link
Owner

Choose a reason for hiding this comment

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

That's also getting out of date, I didn't notice it. It should include the config file used,

maybe the most simple solution is to, in verbose mode, print which config file is used? But I think it would be nicer if each error contained the name of the file

Copy link
Contributor Author

@Andrew15-5 Andrew15-5 Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We don't even have a verbose mode yet. Ok, I'll update this message too. Everything else can be changed after merging this PR.

Copy link
Owner

Choose a reason for hiding this comment

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

We do have a verbose mode, it's when verbose is true, did you rebase work on master?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhhh, I checked on the old binary while being on the latest changes. Yes, there is a verbose option. The description says increase verbosity for non errors. Pretty sure that panic!() is an error.

Copy link
Owner

Choose a reason for hiding this comment

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

I meant we could have the error just complain and the verbose option always print the config file used (even on no error)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh (like an init logs), no that's definitely a separate feature.

Copy link
Owner

Choose a reason for hiding this comment

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

👍

Expand Down