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

feat: add an option to render whitespace in the diff and editor views #751

Merged
merged 4 commits into from
May 12, 2024

Conversation

mwerle
Copy link
Contributor

@mwerle mwerle commented Apr 28, 2024

** UPDATE 2024-05-08 **
Refactored the PR; code changes have been minimised and commits have been merged where it made logical sense.

DESCRIPTION:

This PR adds an option to the "Editor" configuration tab to show whitespace. When this option is checked, whitespace is rendered in the diff and blame editor views. This can be very useful to see incorrect whitespaces (eg tabs vs spaces) when viewing diffs prior to committing files.

ASSOCIATED EDITS:

The translation files were updated to match the updated source files (using the CMake script).

A new translation entry was added for the new configuration option, and the german translation added.

ISSUES:

  • Fixed. When changing the option, the editor view is not refreshed with the new setting. The same issue occurs with the "Blame margin" setting; all other options refresh the views as soon as they are changed. Unsure why some work and others don't as they all use the same underlying APIs.

QUESTIONS:

  1. Is the option path "editor/view/showWhitespace" ok or should another path be used?
  2. fixed should some/all of these commits be squashed prior to merging?

@mwerle
Copy link
Contributor Author

mwerle commented Apr 28, 2024

Just noticed, when rebasing, I lost/accidentally merged 2 commits.

I initially showed EOL marks as well, but they are quite intrusive. I didn't investigate to see if alternate, less intrusive, glyphs can be shown in Scintilla and instead disabled showing EOL marks. This was a separate commit.

This merged commit is af75f39

@mwerle mwerle force-pushed the feature/editor-show-whitespace branch from 9991b98 to 983200f Compare May 8, 2024 02:58
@Murmele
Copy link
Owner

Murmele commented May 8, 2024

I have this macos problem now as well

@mwerle mwerle force-pushed the feature/editor-show-whitespace branch 2 times, most recently from 4bc7741 to 622a829 Compare May 10, 2024 09:12
@Murmele
Copy link
Owner

Murmele commented May 11, 2024

Can you rebase? I think I fixed the macos problem. For some reason macos 14 does not work

This commit adds a new Settings entry "ShowWhitespaceinEditor"
("editor/view/showWhitespace") to the Settings class, and a new checkbox
in the Editor settings dialog.

This setting defaults to off/false, preserving the current behaviour of
the application.

If this setting is toggled to on, whitespace is rendered in the HunkWidget
and BlameEditor views.
Commit updated translation files following source code changes.

NOTE: This commit does not update the actual translations.
Add german translations for the new "Show whitespace" settings in the Settings dialog.
@mwerle mwerle force-pushed the feature/editor-show-whitespace branch from 622a829 to 37c8034 Compare May 12, 2024 15:03
@Murmele Murmele merged commit 39ccb26 into Murmele:master May 12, 2024
11 checks passed
@lonix1
Copy link

lonix1 commented May 12, 2024

Is this related to #603 ?

@mwerle
Copy link
Contributor Author

mwerle commented May 12, 2024

Is this related to #603 ?

No it wasn't; it was purely because I recently had some issues with different types of whitespace in some source files and I like to be able to visualise it to see what's going on. Being told there's a difference but not seeing what that difference actually is makes it difficult to fix..

#603 sounds like it's a feature-request to make it easier to toggle whitespace significance for diffs; ie, instead of in settings, to have a button or menu item.

@lonix1
Copy link

lonix1 commented May 13, 2024

to be able to visualise it to see what's going on.

Yes this is driving me nuts too! :)

I was wondering what this PR does because I'm using not using the dev release, I thought maybe they're related. Regardless, it seems like a good addition from what I've read above - thanks for working on it!

@mwerle
Copy link
Contributor Author

mwerle commented May 13, 2024

Pictures paint a thousand words ;)

Default (whitespace hidden):
Gittyup - hide whitespace

New option selected (whitespace shown):
Gittyp - show whitespace

@lonix1
Copy link

lonix1 commented May 13, 2024

Ooooh... those little dots? Very nice!

@mwerle
Copy link
Contributor Author

mwerle commented May 13, 2024

And arrows for tabs, I felt the EOL marker was too intrusive, but have since read that it's possible to override the glyph. Still, not a big requirement for me so left it off for now.

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

Successfully merging this pull request may close these issues.

None yet

3 participants