Full functionality 'Footnote Fixup' tool #503

G4OEU · 2024-10-24T13:27:34Z

Does not include inline footnotes which will have a separate tool if that is required.

Implements the fast 2-pass version of 'Move FNs to Paragraphs' which uses marks.

Fixes bug caused by interaction with use of marks when last line of the file is a footnote.

Uses the elegant GG1 alpha() function to convert footnote labels to alphabetic form.

Unlike GG1 it correctly handles 'mid-paragraph' footnotes; that is, footnotes at the bottom of the page that are anchored in the paragraph above which continues on to the following page(s).

Footnote Fixup will work consistently before or after Page Marker removal.

Places FNs above a Page Marker if insert point is immediately below the Page Marker.

Full functionality Footnote Fixup without inline footnotes. Fixes bug when footnote is last line. Uses GG1 alpha() function to convert footnote labels to alphabetic form.

windymilla

Just a few minor code tweaks suggested

windymilla · 2024-10-24T19:29:02Z

src/guiguts/footnotes.py

        self.checker_dialog.remove_entry_current()
        maintext().see(fn_prev_end)
+        self.run_check()
+        # display_footnote_entries()


I believe this was left in during debugging, and could be removed now

windymilla · 2024-10-24T19:29:36Z

src/guiguts/footnotes.py

@@ -122,9 +142,434 @@ def join_to_previous(self) -> None:
            f"{fn_cur_start} -1l linestart", f"{fn_cur_end} +1l linestart"
        )
        maintext().delete(f"{fn_prev_end} -1c", f"{fn_prev_end} lineend")
-        maintext().insert(fn_prev_end, "\n" + continuation_text + "\n")
+        # ORIG maintext().insert(fn_prev_end, "\n" + continuation_text + "\n")


Please remove the commented out line of code

windymilla · 2024-10-24T19:33:04Z

src/guiguts/footnotes.py

+            maintext().insert(end_of_file, "\n")
+
+    def move_footnotes_to_paragraphs(self) -> None:
+        """Implements a button command"""


A more specific docstring would be good here

windymilla · 2024-10-24T19:33:59Z

src/guiguts/footnotes.py

+        display_footnote_entries()
+
+    def move_footnotes_to_lz(self) -> None:
+        """Implements a button command"""


A more specific docstring would be good here

windymilla · 2024-10-24T19:37:07Z

src/guiguts/footnotes.py

+        Copies and deletes the original then reinserts edited version at same place.
+        """
+        assert _the_footnote_checker is not None
+        fn_records = _the_footnote_checker.get_fn_records()


It would be good to group all these together into one "undo" block - other functions could possibly also benefit from it. That way the user could undo an action with a single click.

windymilla · 2024-10-24T19:40:15Z

src/guiguts/footnotes.py

+        fn_index_style values are 'number', 'letter' or 'roman'
+        """
+        assert _the_footnote_checker is not None
+


Probably another one that would benefit from and "undo" block

windymilla · 2024-10-24T19:40:51Z

src/guiguts/footnotes.py

+            )
+            fn_start = f"{fn_record.start.index()} linestart"
+            # Replace (first line of) footnote using new label value.
+            # maintext().delete(f"{fn_start} linestart", f"{fn_start} +1l linestart")


Please remove the commented out code

Skip first 200 lines before inserting chapter LZs. 'Set Anchor' checks that a footnote is selected. Tell user to 'reindex' if duplicate labels found when attempting to move to LZs. Adjust layout of frames to minimise blank space in display of buttons. Ordinary Frame used instead of Labelframe. Frames/Buttons expand to fill the width of the dialog. Fix sundry code review comments. Add/improve error messages.

srjfoo · 2024-11-06T08:09:58Z

With a feature as complex as this, I like to test against several source files, and I'm not quite finished yet, but have a couple of comments.

I was surprised that even with chapter landing zones, I was required to re-index, so I went back and checked GG1, and it does not require re-indexing for chapter landing zones.

Also, based on the widget, GG1 also allows the PPer to customize individual footnotes -- they don't all have to be the same kind (numeric, alpha, Roman), which appears to be the case here.

windymilla · 2024-11-06T16:01:40Z

I was surprised that even with chapter landing zones, I was required to re-index, so I went back and checked GG1, and it does not require re-indexing for chapter landing zones.

I don't think I've worked on a book that needs separate FN numbering for each chapter, but @G4OEU has a wider range of PPing experience than me, and @srjfoo sees a lot of different books when marking books as posted & other tasks. If you two think it's a necessary feature, to permit the proofed FN numbers to remain, instead of reindexing, I'd be happy with that.

Also, based on the widget, GG1 also allows the PPer to customize individual footnotes -- they don't all have to be the same kind (numeric, alpha, Roman), which appears to be the case here.

@G4OEU & I had a conversation about that feature during the development of Footnote Fixup. My opinion was that it would be fine for us not to have it, at least to begin with. Since it's a rare occurrence to need it, and you can get round it with a bit of manual S/R to just work on one style of footnotes at a time, maybe we could wait and see if there's a call for it?

srjfoo · 2024-11-06T17:15:02Z

I was surprised that even with chapter landing zones, I was required to re-index, so I went back and checked GG1, and it does not require re-indexing for chapter landing zones.

I don't think I've worked on a book that needs separate FN numbering for each chapter, but @G4OEU has a wider range of PPing experience than me, and @srjfoo sees a lot of different books when marking books as posted & other tasks. If you two think it's a necessary feature, to permit the proofed FN numbers to remain, instead of reindexing, I'd be happy with that.

I'd like to see input from PPers who do that sort of project.

I'd actually like to see the ability to reindex by chapter, so that if the PPer desires, the numbering can start over at 1 by chapter, for the sources where the numbering restarts by page, but I don't think that's any more useful than just reindexing the whole document. If I were actively PPing, I'd think that I'd want to be able to match the scan where it's possible to do so, though, so not forcing full-document reindexing if the renumbering starts by chapter in the original makes sense to me.

Also, based on the widget, GG1 also allows the PPer to customize individual footnotes -- they don't all have to be the same kind (numeric, alpha, Roman), which appears to be the case here.

@G4OEU & I had a conversation about that feature during the development of Footnote Fixup. My opinion was that it would be fine for us not to have it, at least to begin with. Since it's a rare occurrence to need it, and you can get round it with a bit of manual S/R to just work on one style of footnotes at a time, maybe we could wait and see if there's a call for it?

I'm okay with that approach.

srjfoo · 2024-11-07T18:07:17Z

After multiple distractions, finally getting back to this. Looking at the screenshot below, notice a couple of things:

The "Re-run" button is off to the right all by itself. None of the other buttons extends below the re-run button.
The longer button labels may be clipped, depending on how wide the results window is. This is just an fyi as something to keep an eye on until the dialog settles into a final form.

windymilla · 2024-11-08T20:17:13Z

The "Re-run" button is off to the right all by itself. None of the other buttons extends below the re-run button.

The longer button labels may be clipped, depending on how wide the results window is. This is just an fyi as something to keep an eye on until the dialog settles into a final form.

I have a couple of ideas to help with these, unless you've already tackled them @G4OEU

Since the rearrangement of the old multiple frames version, the Frame frame now only has one item inside it: fixit_frame. So you could get rid of frame and make fixit_frame a direct child of header_frame instead:
fixit_frame = ttk.Frame(self.checker_dialog.header_frame)
To make that frame extend underneath the Re-run button, you can add columnspan=2 to its grid command (Note to self: I should really fix checkers.py, so that we don't need to add that to every new checker dialog we create, and get the padding sorted so it aligns with the RH edge of the Re-run button without needing to add extra padding every time - issue added.)
To make all the buttons get an equal proportion of any stretch or squeeze of the dialog width (and hence minimise the clipping @srjfoo saw) set the weight=1 for all 4 columns - there are some near duplicate calls to column_configure and at least one of them sets the weight to zero. The duplicates can be removed - weight doesn't need setting before each button is added, just once for each column, not every row of every column.

1. Uses maintext().undo_block_begin() in appropriate places 2. Fixes a very rare bug in reindexing that can occur when there is more than one anchor on a line 3. Ensures 'Move FNs to Paragraphs' inserts FNs before the 'page marker' when end of paragraph blank line immediately follows the 'page marker'.

G4OEU · 2024-11-14T22:57:17Z

Have just committed changes and pushed them to origin but am getting some sort of failure message from GitHub (see above).

The list of changes in that commit omitted to say that buttons now extend under the 'Re-run' button and that, as part of that fix, 'weight=1' is set just once for the four button columns. The duplicated code that set weights has been removed.

To add some detail about the rare reindexing bug, I've modified the reindex() function to set temporary marks for the hilite start/end columns. They are removed at the end of the function. This fixed a rare bug that could occur when there are two or more anchors on the same line. If the label of an earlier anchor is lengthened on a line during reindexing then you cannot rely on the hilite start/end columns in the records of other anchors on that same line. I had dealt with that possibility and it worked fine except for a contrived example of anchor/footnote labelling which highlighted a bug. Using marks fixed the issue.

1. Deleted commented code that had been spotted during review. 2. Uses 'maintext().undo_block_begin()' in appropriate places. 3. Fixes rare reindexing bug. 4. Ensures that FNs are inserted above a page break when end of paragraph blank line is immediately below the page break. 5. Buttons extend below the 'Re-run' button and 'weight=1' is correctly applied to the four columns.

windymilla

I think all the points I'm aware of have now been addressed, either via changes to the code, or in new issues (#515, #526, #527)

srjfoo · 2024-11-17T09:17:27Z

src/guiguts/footnotes.py

+        self.checker_dialog.select_entry_by_index(dialog_entry_index)
+
+    def autoset_chapter_lz(self) -> None:
+        """Insert a 'FOOTNOTES:' LZ header line before each chapter break.


Discrepancy between GG2 and GG1 (moving footnotes to chapters).

In the first screenshot. Notice the position of fn 1 in the text file (split screen is irrelevant in this screenshot, ignore fn 2).

GG1 does not place an LZ following the chapter title, but above the following chapter header.

I'm not sure what the difference is (I may need to look at the file more closely later), but notice that in this second screenshot, the footnote anchor is in the same place, but in this case, the footnote is more correctly moved to the end of the chapter:

A couple of comments/questions.

I understand that GG1 places the landing zones roughly the same way, but to me, looking at where the landing zones are placed, they're not where I would expect them to be. That is, yes, they're placed before the chapter header by four blank lines, but, if the chapters all start on a new page, that means that the footnotes for one chapter are actually on the same page as the beginning of the following chapter. Which seems to me to be problematical if a file has page numbers associated with it. Am I missing something that's a part of PP'ing?

I expected "Tidy Footnotes" to remove the unwanted "FOOTNOTES:" tags. It didn't for me in GG2. I checked GG1, and it did. Is this a Mac issue, or is it again something I should be doing that I'm not aware of?

A couple of comments/questions.

I understand that GG1 places the landing zones roughly the same way, but to me, looking at where the landing zones are placed, they're not where I would expect them to be. That is, yes, they're placed before the chapter header by four blank lines, but, if the chapters all start on a new page, that means that the footnotes for one chapter are actually on the same page as the beginning of the following chapter. Which seems to me to be problematical if a file has page numbers associated with it. Am I missing something that's a part of PP'ing?

I expected "Tidy Footnotes" to remove the unwanted "FOOTNOTES:" tags. It didn't for me in GG2. I checked GG1, and it did. Is this a Mac issue, or is it again something I should be doing that I'm not aware of?

After touching base with @windymilla, I learned a couple of things:

Regarding the unused LZ removal, that should happen when the footnotes are moved to LZs.

Tidy Footnotes is only supposed to be used on the text file, after Text and HTML are split.

So, I was indeed misunderstanding. However, the unused LZs should be removed.

Re. removing unused LZ headers, the current (and new) Guiguts Manual agrees with you and reminds PPers that:

"If you used Autoset Chap. LZ, there may be unused Landing Zones at the ends of chapters with no footnotes. (Recall that Landing Zones are the word FOOTNOTES:). You probably will want to delete the unused Landing Zones. Similarly, if you moved all of the footnotes to the end of the document and later moved them to follow the paragraphs that referenced them, remember to delete FOOTNOTES: at the end of the document."

I think the original point @srjfoo made is still relevant: although the manual does say you should check for unused LZs, in fact GG1 automatically removes unused LZs immediately after the "Move FNs to LZs" operation. Looking at the code (footnotes.pm, line 1017) it determines this by looking 2 lines below the FOOTNOTES heading and checking if the line starts with [ (which it would if there's a footnote there).

@srjfoo and you are right about this. I'll add that fix along with the other issue that @srjfoo identified.

@G4OEU -- many thanks. I just want to let you know that your effort is very much appreciated! Filling out Footnote Fixup is a major undertaking!

It's probably both an advantage and a disadvantage as a tester, that my perspective is that of someone who has done some post-processing, long ago, but not enough to remember the whole procedure and what did and didn't work in GG1 in the days of 0.2.4. I'll let @windymilla pass judgment on that (😁). It does encourage me to ask questions when something doesn't make sense to me, though!

G4OEU · 2024-11-18T09:01:34Z

@srjfoo has pointed out that in the case of moving FNs to a LZ at the end of each chapter that "yes, they're placed before the chapter header by four blank lines, but, if the chapters all start on a new page, that means that the footnotes for one chapter are actually on the same page as the beginning of the following chapter".

This should be fixed in the same way that is done when FNs are move to paragraph ends. In the latter case care is taken to ensure that, where a paragraph ends at the bottom of a page, any FNs anchored in that paragraph are inserted before the page break so that they are placed on the same page as the paragraph.

windymilla · 2024-11-18T15:56:23Z

@srjfoo has pointed out that in the case of moving FNs to a LZ at the end of each chapter that "yes, they're placed before the chapter header by four blank lines, but, if the chapters all start on a new page, that means that the footnotes for one chapter are actually on the same page as the beginning of the following chapter".

This should be fixed in the same way that is done when FNs are move to paragraph ends. In the latter case care is taken to ensure that, where a paragraph ends at the bottom of a page, any FNs anchored in that paragraph are inserted before the page break so that they are placed on the same page as the paragraph.

From my brief trial, if we did that, it would be an improvement on GG1's behavior.

1. Place 'Chapter End' FNs before Page Markers/page marks when the latter are at the top of a 4-line chapter break; 2. Fix bug when chapter breaks occur mid-page rather than starting a new page; 3. Remove unused LZ headers after FNs are moved to LZs.

Fix Sphinx warning about docstring issue in add_blank_line_at_eof().

Fix docstring warning from sphinx-build.

windymilla

Looks good to me now - thanks!

srjfoo · 2024-12-04T08:19:38Z

src/guiguts/footnotes.py

+        Before inserting the first LZ header, skip the first 200 lines of the
+        file if it is longer than 200 lines. This avoids inserting LZ headers
+        at pseudo paragraph breaks in front matter pages.


This is true of most projects with footnotes, but I feel pretty uncomfortable with this as a hard threshold (even if it's also true of GG1), without some way for those with projects where the footnotes start before that threshold to deal with it easily. One of the test projects I've downloaded has the first footnote anchor before line 200, even though the footnote itself is after; another has two footnotes (both anchor and footnote, before line 200). I browsed the PP pool, and found another that had first footnote and anchor preceding line 200.

Since this is the same as GG1, I'd be happy for it to be merged with the 200 threshold. However, it occurs to me that we could maybe improve on GG1's algorithm by starting chapter LZs after the location of the first footnote, rather than after a fixed line number. As I said, happy for that to go into future footnote improvements, if not in this PR.

This should be simple to fix. I will add to this PR the @windymilla suggestion of starting chapter LZs after the first footnote rather than after the first 200 lines. Thank you @srjfoo for highlighting the issue.

srjfoo

I think this can be classified as an off-by-one problem, maybe. What appears to be happening is that the chapter LZs are being placed one line above the last line of the previous chapter. I've only checked a project with chapters that start on a new page, so I don't know what happens for chapters with mid-page chapter heads.

Edit: -- the spotlighting is because I did a simple search for four returns (probably should have been five, but, whatever 😁). I did check all of the chapters in this book and they all had the same issue.

srjfoo · 2024-12-05T09:06:17Z

src/guiguts/footnotes.py

+            # NB This corrects a bug found by @sjfoo when chapter breaks occur mid-page
+            #    rather than starting a new page. A footnote at the bottom of the page


I appreciate the thought, but I just figured out that chapter-end LZs were slightly misplaced -- @windymilla is the one who figured out the potential mid-chapter footnote misplacement.

srjfoo · 2024-12-09T06:22:07Z

Update -- I was able to quickly find a project that had a handful of footnotes, and chapters mid-page. The anomaly shown above did not happen for chapters mid-page.

1. Fix review issues. 2. Fix 'Move FNs to Paragraph' bugs. 3. Fix issues identified in tests against GG1 output. In particular, ensure that bugs in the GG1 version are not seen in this GG2 version. 4. Ensures that when footnotes are moved to LZs or to paragraph end they are always placed on the correct page. This is a bug fix of the current Footnote Fixup and does not implement mixed label types and related changes to re-indexing. However it provides all the functionality needed to correctly process footnotes in all but a few unusual cases.

srjfoo

It looks good to me, with one caveat that I haven't checked out fully, and have only seen the effect on chapter LZs.

The book that I tested this on had a Book 1 and a Book 2, with a half dozen-ish chapters in each book. If I added page markers before setting the chapter LZs, I got one LZ below the Book 2 header and one at the end of the book (which I think is automatic?). I took the page markers out (after backing out the LZs) and re-ran it and got the correct placement.

I think we should go ahead and get this PR in and then tag the issue (if y'all would, please, test and see if you get the same results, and I'll re-test later, as well).

windymilla · 2024-12-18T11:40:36Z

The book that I tested this on had a Book 1 and a Book 2, with a half dozen-ish chapters in each book. If I added page markers before setting the chapter LZs, I got one LZ below the Book 2 header and one at the end of the book (which I think is automatic?). I took the page markers out (after backing out the LZs) and re-ran it and got the correct placement.

Assuming by "added page markers", you mean File-->Project-->Add Page Marker Flags, then I don't think we need to do anything, apart from document in the manual that you shouldn't try to do anything (significant) when Page Marker Flags are visible. (Apart from expert usage) they are just for transfer to GG1 and back. I'll add a sentence to the manual now. ETA: Done

srjfoo · 2024-12-18T18:02:13Z

The book that I tested this on had a Book 1 and a Book 2, with a half dozen-ish chapters in each book. If I added page markers before setting the chapter LZs, I got one LZ below the Book 2 header and one at the end of the book (which I think is automatic?). I took the page markers out (after backing out the LZs) and re-ran it and got the correct placement.

Assuming by "added page markers", you mean File-->Project-->Add Page Marker Flags, then I don't think we need to do anything, apart from document in the manual that you shouldn't try to do anything (significant) when Page Marker Flags are visible. (Apart from expert usage) they are just for transfer to GG1 and back. I'll add a sentence to the manual now. ETA: Done

Yes, that's what I meant, thanks. And, of course, I had what I thought was a good reason for having them visible, but, as you say, it was not what I'd consider a normal situation.

G4OEU added 2 commits October 18, 2024 22:01

Completes 3-pass version of move to paras

352367a

Places FNs above a Page Marker if insert point is immediately below the Page Marker.

Review footnotes.py

ad067a0

Full functionality Footnote Fixup without inline footnotes. Fixes bug when footnote is last line. Uses GG1 alpha() function to convert footnote labels to alphabetic form.

windymilla self-requested a review October 24, 2024 19:26

windymilla requested changes Oct 24, 2024

View reviewed changes

windymilla linked an issue Oct 25, 2024 that may be closed by this pull request

Footnote fixup #50

Closed

windymilla requested a review from srjfoo November 1, 2024 16:32

This was referenced Nov 15, 2024

Improve handling of multiple footnote anchors #515

Open

Support per-chapter numbering for footnotes #526

Open

Consider support for different styles of footnote in the same file #527

Open

windymilla approved these changes Nov 15, 2024

View reviewed changes

srjfoo reviewed Nov 17, 2024

View reviewed changes

G4OEU added 3 commits November 28, 2024 17:07

Update footnotes.py

c5f11c3

1. Place 'Chapter End' FNs before Page Markers/page marks when the latter are at the top of a 4-line chapter break; 2. Fix bug when chapter breaks occur mid-page rather than starting a new page; 3. Remove unused LZ headers after FNs are moved to LZs.

Update footnotes.py

3bf7a99

Fix Sphinx warning about docstring issue in add_blank_line_at_eof().

Update footnotes.py

16c4d88

Fix docstring warning from sphinx-build.

windymilla requested review from srjfoo and windymilla November 29, 2024 16:03

windymilla approved these changes Nov 29, 2024

View reviewed changes

srjfoo reviewed Dec 4, 2024

View reviewed changes

srjfoo suggested changes Dec 9, 2024

View reviewed changes

srjfoo approved these changes Dec 18, 2024

View reviewed changes

windymilla merged commit 06b9cbe into DistributedProofreaders:master Dec 18, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Full functionality 'Footnote Fixup' tool #503

Full functionality 'Footnote Fixup' tool #503

G4OEU commented Oct 24, 2024 •

edited

Loading

windymilla left a comment

windymilla Oct 24, 2024

windymilla Oct 24, 2024

windymilla Oct 24, 2024

windymilla Oct 24, 2024

windymilla Oct 24, 2024

windymilla Oct 24, 2024

windymilla Oct 24, 2024

srjfoo commented Nov 6, 2024

windymilla commented Nov 6, 2024

srjfoo commented Nov 6, 2024

srjfoo commented Nov 7, 2024

windymilla commented Nov 8, 2024

G4OEU commented Nov 14, 2024

windymilla left a comment

srjfoo Nov 17, 2024

srjfoo Nov 17, 2024

G4OEU Nov 18, 2024

windymilla Nov 18, 2024

G4OEU Nov 18, 2024

srjfoo Nov 18, 2024

G4OEU commented Nov 18, 2024 •

edited

Loading

windymilla commented Nov 18, 2024

windymilla left a comment

srjfoo Dec 4, 2024

windymilla Dec 4, 2024

G4OEU Dec 4, 2024

srjfoo left a comment •

edited

Loading

srjfoo Dec 5, 2024

srjfoo commented Dec 9, 2024

srjfoo left a comment

windymilla commented Dec 18, 2024 •

edited

Loading

srjfoo commented Dec 18, 2024

		# NB This corrects a bug found by @sjfoo when chapter breaks occur mid-page
		# rather than starting a new page. A footnote at the bottom of the page

Full functionality 'Footnote Fixup' tool #503

Full functionality 'Footnote Fixup' tool #503

Conversation

G4OEU commented Oct 24, 2024 • edited Loading

windymilla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srjfoo commented Nov 6, 2024

windymilla commented Nov 6, 2024

srjfoo commented Nov 6, 2024

srjfoo commented Nov 7, 2024

windymilla commented Nov 8, 2024

G4OEU commented Nov 14, 2024

windymilla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

G4OEU commented Nov 18, 2024 • edited Loading

windymilla commented Nov 18, 2024

windymilla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srjfoo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srjfoo commented Dec 9, 2024

srjfoo left a comment

Choose a reason for hiding this comment

windymilla commented Dec 18, 2024 • edited Loading

srjfoo commented Dec 18, 2024

G4OEU commented Oct 24, 2024 •

edited

Loading

G4OEU commented Nov 18, 2024 •

edited

Loading

srjfoo left a comment •

edited

Loading

windymilla commented Dec 18, 2024 •

edited

Loading