Fix computed values to be computed after all set_options

[ghost]

What this pr does

• Make "true-color = ***" from gitconfig works

How it was

It was not working from original source.

So I make it works.

This still give prior to the config from cli. Also 24-bit-color can still work.

Motivaiton

I'm trying to integrate delta inside tig.

#689

However, if you allow user to use RGB, it's almost impossible inside tig since it uses ncurses. So I'm trying to limit only 8bit colors. Then I found this bug.

[dandavison]

Hi again @ulwlu! Great, thanks for this. I've pushed one commit to your branch adding a test that is intended to reproduce the bug that you are fixing. But there's one thing I'm confused about:

My test fails on master and passes on your branch. However my test tests true-color = never whereas the new code you've added is inside a if opt.true_color == "auto" statement. I haven't had time yet to figure out why my test passes on your branch. Would you be able to look at the test, and check everything makes sense, and add any further test cases you think are needed?

[ghost]

Hi @dandavison thank you for your review.

This is bacause cli.rs set opt.true-color to 'auto' if '--true-color' option is not given. That is the default value given by struct-opt.

I think it's little bit confusing, maybe we can remove the default value and change match condition inside set_true_color function.

[dandavison]

true_color is present in the main call to set_options! on l.182. I think we need to know why that call is not working -- it feels wrong to make two set_options! calls for true_color.

[ghost]

Oh I see, and sorry. I missed that and found opt.true_color is set correctly (not exact correct because it's not computed into bool) from gitconfig after set_options. I'm maybe going to fix.

[dandavison]

This is bacause cli.rs set opt.true-color to 'auto' if '--true-color' option is not given. That is the default value given by struct-opt.

Thanks for explaining that!

[ghost]

Thanks to your pointing out, I found some configurations which is to be computed depends on opt.*** value must be after set_options which gets value from gitconfig. It is because gitconfig value will never be computed.

I'm going to think what way is good.

[ghost]

@dandavison

Hi I fixed and change this PR's purpose.

• Change ComputedValues's computing place after all set_options
  • It is because some of them depends on opt.** value but they have chance to miss gitconfig values
• The ComputedValues which have a default value and computed before set_options were going wrong.
  • The ComputedValues which have a default value: paging-mode, inspect-raw-lines, and true-color
  • The ComputedValues which computed before set_options: true-color
  • So, only true-color was going wrong

I move set_width after set_options as well, so this func now doesn't need to set_options inside it.

• I added a test for these computed values are setting correctly into config.
• I removed an unused ComputedValue.

[dandavison]

Awesome, thanks very much for these fixes @ulwlu. I did some manual testing to check that width worked when set as a command-line option, and in gitconfig, and via git -c delta.width=xxx ....

[ghost]

@dandavison thanks!

Btw, integrating tig with delta is almost finished. Now I'm fixing little bugs.

jonas/tig#1140

[dandavison]

Btw, integrating with delta is almost finished. Now I'm fixing little bugs.

Awesome. Those screenshots look really nice. (I just tried compiling but got an error

src/line.c:245:3: error: implicit declaration of function 'init_extended_pair' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
                init_extended_pair(++cnt, fg, bg);

)

[ghost]

Umm, init_expanded_pair is only available with ncurse upper than version 6.1, so compiling option may get wrong (such as linking with different path).

I also strrugled at that building. If this get merged jonas will release the new binary anyway, so maybe trying after that is easier...

[dandavison]

OK thanks (my Mac has 5.4, but I saw your instructions for building and linking against ncurses).