🐛 width option is not honored in git config sections

When side-by-side is on, it gets really wide.
It would be great if I could set max-column( sorry if there is already. I look for document and issues, but couldn't find.)

[dandavison]

Hi @Ryuta69, I think width does what you want.

@dandavison

Thank you for your reply.

I set width, however, it doesn't affect any size.
My git config is like this. minimal git config is this.

[core]
    pager = delta

[delta]
    width = 100
    line-numbers = true
    side-by-side = true
    hunk-header-style = omit

[interactive]
    diffFilter = delta --color-only

[dandavison]

Thanks @Ryuta69. You're right, that's not working as intended. Can you use this as a workaround:

[core]
    pager = delta --width 100

[delta]
    ...

You will see the background colors in the right panel extend to the full width, because they are painted by the terminal emulator:

There is not currently a way to make the background colors terminate at the --width column. I hope that's OK for you!

@dandavison
ok! it's very fine and great.
Thank you for your reply!

@dandavison
Oh, also, I would like to fix this issue.
Because I had sent PR to this repository before, but it was failed...

Is it fine with you?

[dandavison]

Oh, also, I would like to fix this issue.

That would be great! I'll re-open this issue. I think the place to start would be looking at the call to set_widths here:

delta/src/options/set.rs

Line 84 in c573382

set_widths(opt);

Is it as simple as moving that call to before features::make_builtin_features()? (Or was there some reason I had it after? :) )

@dandavison
Thank you for your advice!

I already fixed config.rs/main.rs to accept gitconfig value and refactor fn set_widths(opt: &mut cli::Opt) to get git_config: &mut git_config::GitConfig as arg. I will send PR in a few days.

Is it as simple as moving that call to before features::make_builtin_features()? (Or was there some reason I had it after? :) )

I think it used to be called before features::make_builtin_features() at eb202089#diff-8b49f2774c51a77ad662f1c88bfd1579R71 , however, you placed features::make_builtin_features() at early position at this commit e65db80b on purpose to avoid a bug.

set_widths itself seems not to have any problems to be placed before features::make_builtin_features(), but I think it would be better to place features::make_builtin_features() in the very beginning of this function, so that everyone could avoid any bugs......

just IMO. I was not going to move, but if you think it would be simpler, I'll move.

[dandavison]

On further thought it seems that at a high level what we want is

1. The code in set_options() is allowed to do its job: it gathers up all the user settings from the command line, and from git config sections, and figures out what raw value the user has supplied for width (the value might be an integer, or it might be the string "variable").

2. Afterwards, the logic in set_widths() is executed, which interprets the user value, and stores its interpreted values in the .computed struct.

So I think what I said above about moving set_widths to earlier was wrong. I'm happy to wait and see your PR!

Have you already looked into adding a unit test? If not, here's a start:

    #[test]
    fn test_width_in_git_config_is_honored() {
        let git_config_contents = b"
[delta]
    width = 7
";
        let git_config_path = "delta__test_width_in_git_config_is_honored.gitconfig";

        let opt = integration_test_utils::make_options_from_args_and_git_config(
            &[],
            Some(git_config_contents),
            Some(git_config_path),
        );

        assert_eq!(opt.computed.decorations_width, cli::Width::Fixed(7));

        remove_file(git_config_path).unwrap();
    }

Great to hear that! Yes, I will.
I look into the unit test, and will make one! Thank you!