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

Add ruler to display column numbers #573

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

tangledhelix
Copy link
Collaborator

Adds a ruler along the top of each text view to display column numbers, highlighting current column number.

Adds a user preference to prefs panel (defaulting False) for control of this feature.

Fixes #61

Testing Notes

  • Turn on column numbers in the preferences pane
  • Ruler should scroll horizontally with its text view
    • Using mousewheel / trackpad scroll in text area
    • Using scrollbar
  • I suggest testing at narrow and wide widths
  • Also suggest testing just at the point where the horizontal scrollbar appears when you narrow the window just enough
  • The ruler maxes out at 500 columns
  • The ruler is only supposed to be drawn as long as the longest visible line
  • UNLESS that line is narrower than the window. Then it pads to the window width.
  • Colors should dynamically change in Default theme when OS dark mode changes
  • When using split view, the focused view should have highlighted column just like the line numbers; the non-focused view should have the same de-emphasized coloring as the line numbers.

@windymilla when I use the Dark theme the colors are broken. Maybe you can tell me why? I haven't been able to find that one. It does work in Default theme when the OS dark mode is adjusted (at least on Mac). And the Light theme works.

@tangledhelix tangledhelix force-pushed the ruler branch 2 times, most recently from d7cbfd6 to cb1e8e5 Compare December 15, 2024 08:52
Copy link

@rtonsing rtonsing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I use dark theme, the column bar text remains black. I'm not using dark mode in the OS (Win 11).

@windymilla
Copy link
Collaborator

windymilla commented Dec 15, 2024

  1. Regarding theme colors, to change the text color ground of a Text widget, you need
    self.configure(foreground=themed_style().lookup("TButton", "foreground"))
    like you do for the background
  2. The following will end up with two different ruler background colors (peer is incorrect) as seen in screenshot
    image
    a. Run guiguts --nohome in order to get default behavior
    b. Turn on the column numbers
    c. Split the text window
  3. The ruler and text are not quite aligned (may be related to this comment in GG1 - see screenshot of file with copy of ruler characters as first line of text:
        -bd        => 2,            # border doesn't show, but keeps it aligned with main text window which has border
        -relief    => 'flat',

image
4. The following two screenshots are of a file that just has one line:
····▾····1····▾····2
Make the window narrow enough that you need to scroll to see the start/end of the line. The first is what you get if you scroll to the left extreme, using the scrollbar, and the second is what you get if you scroll to the right extreme. My window is only 1 line tall, but that isn't relevant. Note how the ruler misaligns to the left in one screenshot and to the right in the other.
image
image
In fact, if you scroll completely to the right, then slightly wiggle the scrollbar left & right, you can see the text moving, but not the ruler. Maybe there's some padding or something in the main text that isn't in the ruler?

@windymilla
Copy link
Collaborator

  1. In GG1, you can turn the line/column numbers on/off via different clicks on the first status bar button (line.col). In GG2, you can turn line numbers on/off like that with Shift-left. I suggest using Shift-right for column numbers on/off. I think it's the kind of thing that would be good to be able to switch on/off quickly without going to prefs, because the ruler can be a bit distracting, so being able to just turn it on for a quick measurement/alignment then off again would be useful. Do you agree?

@windymilla
Copy link
Collaborator

windymilla commented Dec 15, 2024

I was able to get focus into the ruler and type stuff: see screenshot.
I've been able to do it a few times, just clicking the three mouse buttons (maybe scrolling the wheel too?)- I'll try to get a failsafe way to do it

Screenshot 2024-12-15 194747

Note that clicking back in the main text window caused the ruler to be redrawn correctly, so it isn't a big issue.

@windymilla
Copy link
Collaborator

Tracked the focus in ruler issue down - it's the event "ButtonRelease-2".
To be safe, I would bind buttons 1 to 5 (I think the Tk on Linux/X-windows uses buttons 4 & 5 as a way of signalling the scroll wheel)
and bind the press, release and motion, so

            "Button-1",
            "ButtonRelease-1",
            "B1-Motion",
            "Button-2",
            "ButtonRelease-2",
            "B2-Motion",
            "Button-3",
            "ButtonRelease-3",
            "B3-Motion",
            "Button-4",
            "ButtonRelease-4",
            "B4-Motion",
            "Button-5",
            "ButtonRelease-5",
            "B5-Motion",
            "MouseWheel",

@srjfoo
Copy link
Member

srjfoo commented Dec 16, 2024

I'm still poking, but thought I'd share this

image

macOS, dark theme, with GG2 switched to light theme. The colored highlight shows the active pane, the grey shows the inactive pane. The issues I see are that (at least for Macs (don't know about Windows),

  • the background color for the ruler looks about as I'd expect, but the text color follows the OS setting, and is white.
  • The line numbers, on the other hand, follow the theme setting, but the line number highlight (as with the column number highlight) follows the OS, and so is dark instead of light. So

The highlighted line number doesn't show up really well , though the rest of the line numbers do, and the highlighted column number shows up really well, but the rest don't.

(For this test, the font size was set to 60 so that I could do a better job of determining by eye how well things were synced without zooming the screen. In this screenshot, there are no alignment issues between the two panes.)


If I use --nohome, the ruler doesn't re-size. In fact, with size set to 60, you can only see an empty bar. I see the ruler background color issue with --nohome with the default theme, but not with light or dark. And without --nohome, I don't see any problems with the ruler background color on any of the themes.


  1. The following two screenshots are of a file that just has one line: ····▾····1····▾····2 Make the window narrow enough that you need to scroll to see the start/end of the line. The first is what you get if you scroll to the left extreme, using the scrollbar, and the second is what you get if you scroll to the right extreme. My window is only 1 line tall, but that isn't relevant. Note how the ruler misaligns to the left in one screenshot and to the right in the other.
image

In fact, if you scroll completely to the right, then slightly wiggle the scrollbar left & right, you can see the text moving, but not the ruler. Maybe there's some padding or something in the main text that isn't in the ruler?

Padding sounds like a good guess. For what it's worth, I was able to get them to align (or a close approximation thereof) by making minute changes in the window width untill the window was the exact width of a multiple of the character width (or a close approximation thereof 😁 ).


Just wondering why the difference between with and without --nohome as far as the theme color is concerned. Seems weird to me.

@tangledhelix
Copy link
Collaborator Author

When I use dark theme, the column bar text remains black. I'm not using dark mode in the OS (Win 11).

The coloring update should be fixed now. There is still a border around the ruler (which is a layout issue, not a padding issue, but it's prominent and distracting in dark theme, at least for me). Will look at the border separately.

Copy link

@rtonsing rtonsing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems OK now in my limited testing. Note when Windows "Color mode" is set to "Dark", then GG2 theme "Default" still appears the same as "Light", don't know if this is expected.

@tangledhelix
Copy link
Collaborator Author

  1. Regarding theme colors, to change the text color ground of a Text widget, you need
    self.configure(foreground=themed_style().lookup("TButton", "foreground"))
    like you do for the background

This is fixed, thanks for the pointer!

  1. The following will end up with two different ruler background colors (peer is incorrect) as seen in screenshot

This is fixed; now theme_change() is run at widget initialization too.

  1. The ruler and text are not quite aligned (may be related to this comment in GG1 - see screenshot of file with copy of ruler characters as first line of text:

This was a regression bug I introduced while laying out the grid; I added a padx on the two text editor widgets. Removed the padding, and now the widgets align and text is back where it ought to be. While re-testing I instead moved some padding to the ruler to more precisely align with the text. At least, it aligns quite well on Mac, using DP Sans Mono or Courier New; I didn't exhaustively test every font I could think of.

This did uncover another bug, which is that the ruler doesn't dynamically resize when the font size changes. I'll look into that.

  1. The following two screenshots are of a file that just has one line: ····▾····1····▾····2 Make the window narrow enough that you need to scroll to see the start/end of the line. The first is what you get if you scroll to the left extreme, using the scrollbar, and the second is what you get if you scroll to the right extreme. My window is only 1 line tall, but that isn't relevant. Note how the ruler misaligns to the left in one screenshot and to the right in the other. In fact, if you scroll completely to the right, then slightly wiggle the scrollbar left & right, you can see the text moving, but not the ruler. Maybe there's some padding or something in the main text that isn't in the ruler?

This misalign was also caused by the previously-mentioned padx, so it's also fixed.

@tangledhelix
Copy link
Collaborator Author

This did uncover another bug, which is that the ruler doesn't dynamically resize when the font size changes. I'll look into that.

This is fixed.

@tangledhelix
Copy link
Collaborator Author

  1. In GG1, you can turn the line/column numbers on/off via different clicks on the first status bar button (line.col). In GG2, you can turn line numbers on/off like that with Shift-left. I suggest using Shift-right for column numbers on/off. I think it's the kind of thing that would be good to be able to switch on/off quickly without going to prefs, because the ruler can be a bit distracting, so being able to just turn it on for a quick measurement/alignment then off again would be useful. Do you agree?

Seems reasonable to me; added that.

@tangledhelix
Copy link
Collaborator Author

Tracked the focus in ruler issue down - it's the event "ButtonRelease-2". To be safe, I would bind buttons 1 to 5 (I think the Tk on Linux/X-windows uses buttons 4 & 5 as a way of signalling the scroll wheel) and bind the press, release and motion, so

Okay, added all of these.

@windymilla
Copy link
Collaborator

Seems OK now in my limited testing. Note when Windows "Color mode" is set to "Dark", then GG2 theme "Default" still appears the same as "Light", don't know if this is expected.

@rtonsing - unfortunately, that is just how tkinter behaves. On Macs, "Default" follows the system theme, but not on Windows or Linux. If it had, I probably wouldn't have bothered with themes at all :)

@tangledhelix
Copy link
Collaborator Author

macOS, dark theme, with GG2 switched to light theme. The colored highlight shows the active pane, the grey shows the inactive pane. The issues I see are that (at least for Macs (don't know about Windows),

  • the background color for the ruler looks about as I'd expect, but the text color follows the OS setting, and is white.

That's fixed; or at least it's now the same as line numbers. Further improvement is for #353 (IMHO).

If I use --nohome, the ruler doesn't re-size. In fact, with size set to 60, you can only see an empty bar. I see the ruler background color issue with --nohome with the default theme, but not with light or dark. And without --nohome, I don't see any problems with the ruler background color on any of the themes.

I was able to reproduce the behavior. I don't fully understand the cause, but I theorize that with --nohome in play, preferences aren't persisted in the same way, which probably affects the callbacks.

It probably wouldn't affect anyone but devs and testers. But we could work around it by adding a --home like GG1 has... then you would have a place to persist settings, but without affecting the user's baseline environment.

@tangledhelix
Copy link
Collaborator Author

Seems OK now in my limited testing. Note when Windows "Color mode" is set to "Dark", then GG2 theme "Default" still appears the same as "Light", don't know if this is expected.

The intent is that it looks the same as the line numbers, in all the different modes. Are you seeing that they differ, in that mode?

@tangledhelix
Copy link
Collaborator Author

Seems OK now in my limited testing. Note when Windows "Color mode" is set to "Dark", then GG2 theme "Default" still appears the same as "Light", don't know if this is expected.

The intent is that it looks the same as the line numbers, in all the different modes. Are you seeing that they differ, in that mode?

I see that @windymilla already answered this; so I will disregard as it's an expected behavior.

@tangledhelix
Copy link
Collaborator Author

Squashed and rebased.

@tangledhelix
Copy link
Collaborator Author

There is still a border around the ruler (which is a layout issue, not a padding issue, but it's prominent and distracting in dark theme, at least for me). Will look at the border separately.

Forgot to mention in the comments anywhere, but the border was fixed along the way too (highlightthickness=0).

@windymilla
Copy link
Collaborator

There is still a border around the ruler (which is a layout issue, not a padding issue, but it's prominent and distracting in dark theme, at least for me). Will look at the border separately.

Forgot to mention in the comments anywhere, but the border was fixed along the way too (highlightthickness=0).

There is still a border - see screenshot
image
To fix it, instead of highlightthickness, I think you need borderthickness set to zero. Alternatively, set the relief to tk.FLAT.

@windymilla
Copy link
Collaborator

windymilla commented Dec 16, 2024

I realized it's not very clear in that screenshot. Here is a marked up copy of it. Compare the red-circled boundary between columns and the top left corner with the green-circled boundary between line numbers and top left corner. The former has a thin pale border.
image

@windymilla
Copy link
Collaborator

Also just thought, if you remove the borderthickness, you may need to add 2 pixel extra x-padding, to make it 5 in total.

@windymilla
Copy link
Collaborator

I was able to reproduce the behavior. I don't fully understand the cause, but I theorize that with --nohome in play, preferences aren't persisted in the same way, which probably affects the callbacks

That isn't how --nohome should work. It should be equivalent to the following:

  1. Take safe copy of prefs file
  2. Delete the prefs file
  3. Run GG2
  4. Delete the prefs file
  5. Restore safe copy of prefs file

Although it does mean the setting isn't loaded from the prefs file, that is the same as the first time the user runs the program. So, if we are seeing odd behavior with --nohome it is something we should investigate.
Callbacks should still be called whenever a change is made to a prefs value that has a callback. The prefs should persist for the run of the program and behave identically to withouth --nohome, but the prefs do not persist until the next run of the program.

@windymilla
Copy link
Collaborator

windymilla commented Dec 16, 2024

It probably wouldn't affect anyone but devs and testers. But we could work around it by adding a --home like GG1 has... then you would have a place to persist settings, but without affecting the user's baseline environment.

We could, although as noted above, we shouldn't generally need it. If we want to persist a setting when testing across several runs, then don't specify --nohome. If it's also important to have a clean slate, just rename/delete your prefs file before starting GG.
The GG1 --home arose because of the previous behavior of storing the prefs under the release dir tree.

@tangledhelix
Copy link
Collaborator Author

There is still a border - see screenshot
To fix it, instead of highlightthickness, I think you need borderthickness set to zero. Alternatively, set the relief to tk.FLAT.

I don't see the border on this end. I set borderwidth=0, relief=tk.FLAT, but I don't see any difference from what I saw previously.

Before and after:

Screenshot 2024-12-16 at 3 50 39 PM Screenshot 2024-12-16 at 3 51 26 PM

The debugger suggests those were the defaults anyway:

Before:

self.config()["borderwidth"]
('borderwidth', 'borderWidth', 'BorderWidth', 0, 0)
self.config()["relief"]
('relief', 'relief', 'Relief', <index object: 'flat'>, 'flat')

After:

self.config()["borderwidth"]
('borderwidth', 'borderWidth', 'BorderWidth', 0, 0)
self.config()["relief"]
('relief', 'relief', 'Relief', <index object: 'flat'>, 'flat')

@windymilla
Copy link
Collaborator

windymilla commented Dec 16, 2024

The debugger suggests those were the defaults anyway:

Assuming that was GG2 Dark mode (not OS Dark mode & GG2 default) then it's possible that the style for a Text widget is different on Macs and Windows. The default border width on Windows seems to be 2. It also says here in the section on borderwidth:

The default is two pixels.

So, safest that you're setting it explicitly to zero, but explains why you didn't see it previously.

@windymilla windymilla requested review from windymilla and rtonsing and removed request for rtonsing December 16, 2024 21:17
@windymilla
Copy link
Collaborator

Sorry @rtonsing - misclicked. Didn't mean to re-request a review.

Copy link
Collaborator

@windymilla windymilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Adds a ruler along the top of each text view to display column
numbers, highlighting current column number.

Adds a user preference to prefs panel (defaulting False) for
control of this feature.

Fixes DistributedProofreaders#61
@srjfoo
Copy link
Member

srjfoo commented Dec 17, 2024

Revisiting the "misalignment" between main text and peer, it's still there. But it appears to be that two-pixel issue. Or rather, on macOS, all themes.

image

I'm questioning if it really matters a whole lot. Each text block is internally consistent, and if someone wants to see both the beginning and end of a long line using the split screen, I don't think top-to-bottom alignment is going to matter a lick.


I did this [Shift-ButtonRelease-3], but it's acting weird:

  • shift-click works (toggle line numbers)
  • shift-ctrl-click works (toggle col numbers)
  • shift-(two-finger-tap) - normally equivalent to right-click - sometimes reads as cols, sometimes nums...

I don't have an external mouse handy to try an actual right-click button. The 2-finger tap must have some extra magic we don't know yet. @srjfoo what result do you get?

For the weirdness, maybe occasionally when you think you're using a two-finger tap, you're actually only getting a one-finger tap? I had very predictable results with shift-click (i.e. one-finger tap), and shift-(two-finger-tap), as well as, predictably, shift-ctrl-click. I did have an external mouse handy, and shift-right-click worked just fine.

@windymilla windymilla merged commit 6f2c004 into DistributedProofreaders:master Dec 17, 2024
1 check passed
@tangledhelix tangledhelix deleted the ruler branch December 18, 2024 02:51
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.

Add a ruler
4 participants