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

🐛 Some theme options are not working with --color-only enabled #405

Closed
renard opened this issue Nov 23, 2020 · 11 comments
Closed

🐛 Some theme options are not working with --color-only enabled #405

renard opened this issue Nov 23, 2020 · 11 comments

Comments

@renard
Copy link

renard commented Nov 23, 2020

When running delta in a standalone mode (no git) with --color-only to keep a diff / patch compatible patch some theming options such as --file-style or --hunk-header-style (same with --hunk-header-decoration-style) are not operational:

Screenshot 2020-11-23 at 14 58 17

Screenshot 2020-11-23 at 14 59 53

Is there a way to highlight file headers and hunks delimiter in a specific colour?

Diff file:

--- 1	2020-11-23 12:51:34.434055926 +0000
+++ 2	2020-11-23 12:52:06.242055415 +0000
@@ -11,7 +11,7 @@
                                      style plain --hunk-style plain`.
         --dark                       Use default colors appropriate for a dark terminal background. For more control,
                                      see the other color options.
-    -h, --help                       Prints help information
+    -h, --HELP                       Prints help information
         --highlight-removed          Apply syntax highlighting to removed lines. The default is to apply syntax
                                      highlighting to unchanged and new lines only.
         --keep-plus-minus-markers    Prefix added/removed lines with a +/- character, respectively, exactly as git does.

TIA.

@dandavison
Copy link
Owner

Hi @renard, thanks for this! Short answer: yes, I think there are more improvements we can make to --color-only. We should be able to color all elements as long as we are not making any "structural" changes (e.g. decorations).

Until recently, there had been some bugs causing --color-only to create invalid patch output, when used in combination with certain options. For many delta users this comes up when using git add -p. Thanks to the work of @ulwlu, these are mostly, or perhaps entirely, gone now. See e.g. #320. I think perhaps we can characterize this program of work as follows:

  1. There were bugs relating to invalid patches under --color-only
  2. We introduced much better test coverage and fixed bugs, but erring on the side of aggressively eliminating features from --color-only to ensure that a valid (if vanilla in terms of coloring) patch was produced.
  3. Now that we have good test coverage we should be in a better position to revisit and allow color where possible without allowing any decorations/structural changes to the patch.

cc @ulwlu what do you think?

@ghost
Copy link

ghost commented Nov 24, 2020

Hi, my thought is same.

we should be in a better position to revisit and allow color where possible without allowing any decorations/structural changes to the patch

0-1 are true, maybe I can look into it to make --color-only better. I'm going to find a way to allow color without changing structural. It takes me few days maybe.

@dandavison
Copy link
Owner

Thanks @ulwlu, that would be fantastic if you have time.

@ghost
Copy link

ghost commented Nov 27, 2020

P.S.

Currently, I'm busier than I expected so not working yet.
I'm going to start on this Sunday.

-- P.S. 2020-11-29

Sorry again, but still quite busy with my job.
It will take me a week.

@ghost
Copy link

ghost commented Dec 3, 2020

@dandavison

Hi, I'm currently working on this, however, doesn't end-to-end-test fail with current master?

Even though unit-test succeeds, end-to-end-test fails with below stderr.

make: *** [end-to-end-test] Error 1

--
P.S.

I realized this is a bug from tests.
I'm gonna fix this at first.

@dandavison
Copy link
Owner

Thanks @ulwlu! I noticed this yesterday but didn't have a chance to fix. Fixed now I believe. ed39e4c

Now, what I need to work out is why CI was not failing. Locally, the shell scripts exit with a non-zero error status. cc @MarcoIeni (but you don't have any obligations after all your work! :) )

image

@ghost
Copy link

ghost commented Dec 3, 2020

@dandavison

why CI was not failing

I think CI succeeds because it passes argument.
If we command locally as same as shell of CI, I confirm it works.

@dandavison
Copy link
Owner

@ulwlu Let's discuss in #421

dandavison pushed a commit that referenced this issue Dec 8, 2020
… (#436)

* Add config property of color_only

* Delete force assign raw to file_style when color_only

* Print filemeta in color with rawline when color_only mode

* Cargo fmt

* Add test if file_style with color_only has style

* Add comment about color_only
dandavison pushed a commit that referenced this issue Dec 8, 2020
…only #405 (#437)

* Remove force passing raw to hunk header style when color_only

* Handle hunk header color when color_only

* Add test for hunk header style color when color_only

* Delete fixme because it actually is required

* Add comment for condition that keeps structure when color_only and raw mode
dandavison pushed a commit that referenced this issue Dec 8, 2020
…405 (#438)

* Remove force assing raw to commit style when color_only

* Add test for commit style color when color_only
@dandavison
Copy link
Owner

Fixed in master by @ulwlu in #405 and #443. (Not yet released)

image

hunk-header-style = syntax also works.

@dandavison
Copy link
Owner

@ulwlu' fixes are released now (v0.4.5).

@ghost
Copy link

ghost commented Dec 23, 2020

@dandavison
Thank you very much! Sounds great!

ps
PRs for using termbg and fixing features may not sent in this year since I'm still during treatment of ICL surgery, and planning to travel after that.

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

No branches or pull requests

2 participants