I’ve seen @pmeunier and @flobec creating “rustfmt patches” from times to times. I wonder if we shouldn’t make one last one, then using it systemically before recording any patches.
This way, we will have a more consistent coding style across the pijul source code.
Fortunately, we have hooks to remember to run the script for us:
Create an executable .pijul/hooks/pre-record with the following content:
#!/usr/bin/bash
cargo fmt
definitely. Done in my own repo.
Question: is it possible to have global template hooks like in git, so that whenever I create a pijul repo that hook is automatically copied inside? (obviously in that case it would need to be more generic, as not all projects are in rust)
I had a look at the rustfmt configuration options. I personally prefer the visual options, but I’ve seen the default is block and that it follows one rust official RFC… So if you guys would like to stick to the official guidelines, I am fine with that.
Once we agree, we can add a rustfmt.toml file to the root of the repo, and update the README file accordingly.
If you scroll down, you’ll see that each option has a different default, for example “control_brace_style” is “AlwaysSameLine”, “error_on_line_overflow” is “false”, etc.
Yeah, I was not clear enough. I meant, for the first few configuration options, I prefer visual over block. For instance
fn function(arg1: x,
arg2: y)
over
fn function(
arg1: x,
arg2: y
)
but again, this is more a matter of personal taste.
I strongly prefer the default (C-style) one.
Then, default it is!
Is it okay if I push the “rustfmt patch”, then? Do we agree on using it before recording?
I really like your idea of using this systematically, so yes! Go ahead!
I fear this sentence might come across a bit too forceful in tone:
If your patch does not comply with the coding style enforced by
rustfmt, your patch might be refused as-is.
Maybe it’s better:
Please make sure to comply with the rustfmt coding style before submitting your patches!
You are probably right. I will edit as you suggest tomorrow, or you could always propose the change and I would apply it quickly.
For the record, I just pushed another patch to apply globally rustfmt.
Edit: And I have unrecorded it, at it appears it fires a strange conflict within pijul/src/commands/remote.rs.
Will have to dig… another day.
I’d be interested in debugging this. What did your patch do exactly?
cargo fmt
pijul record -a
The repo looks fine, but clone it and you should see a conflict in remote.rs
–lthms
I can’t reproduce, even if I fix the cargo fmt failure to format pijul::commands::fs_operations.
Maybe we are not using the same version of rustfmt? Or maybe, my pijul repo was in a strange state, or I used a strange pijul, in order to record this patch. I don’t really know.
I will come back to you if I encounter the issue ever again.
Edit: We definitely don’t have the same rustfmt version.
I can’t reproduce either, using rustfmt 0.3.8-nightly
Even though now we are all using rustfmt, I still get new formatting on supposedly already formatted code. I think it’s because we are using different versions of rustfmt.
I will try to oush a rush fly.toml file, see if it does us any good.
Ado, have you installled rustfmt with rustup ? I believe it is the best way for us to stay sync
–lthms
$ cargo fmt --version
rustfmt 0.6.1-nightly (0f8029f 2018-05-06)
Could you try to create a new file rustfmt.toml at the root of the repo, with this content:
max_width = 100
hard_tabs = false
tab_spaces = 4
newline_style = "Unix"
use_small_heuristics = true
indent_style = "Block"
wrap_comments = false
comment_width = 80
normalize_comments = false
format_strings = false
empty_item_single_line = true
struct_lit_single_line = true
fn_single_line = false
where_single_line = false
imports_indent = "Block"
imports_layout = "Mixed"
merge_imports = false
reorder_imports = true
reorder_modules = true
reorder_impl_items = false
type_punctuation_density = "Wide"
space_before_colon = false
space_after_colon = true
spaces_around_ranges = false
spaces_within_parens_and_brackets = false
binop_separator = "Front"
remove_blank_lines_at_start_or_end_of_block = true
combine_control_expr = true
struct_field_align_threshold = 0
match_arm_blocks = true
force_multiline_blocks = false
fn_args_density = "Tall"
brace_style = "SameLineWhere"
control_brace_style = "AlwaysSameLine"
trailing_semicolon = true
trailing_comma = "Vertical"
match_block_trailing_comma = false
blank_lines_upper_bound = 1
blank_lines_lower_bound = 0
merge_derives = true
use_try_shorthand = false
condense_wildcard_suffixes = false
force_explicit_abi = true
use_field_init_shorthand = false
write_mode = "Overwrite"
color = "Auto"
required_version = "0.6.1"
unstable_features = false
disable_all_formatting = false
skip_children = false
hide_parse_errors = false
error_on_line_overflow = false
error_on_unformatted = false
report_todo = "Never"
report_fixme = "Never"
ignore = []
verbose_diff = false
And tell me if it works for you? If it does, I will push a patch to add it, along with a rustfmt patch.
Were you talking to me?
With rustup I now have rustfmt 0.4.1-stable, everything is up to date. Seems it depends on the default toolchain? Must be resolved anyway.
On a related note, is it possible to have a pre-push hook that checks if pijul builds before sending to the nest? I think it would be even more useful than the rustfmt hook.