Got OnePass() to be called reliably when user presses ENTER or TAB, and removed unneeded code changes put into JumpForward() in the last two days. Retranslation made and removed without any error. Test project was not a KBserver sharing one.

adaptitbruce · adaptitbruce · commit bd2539f2c259 · 2024-03-19T14:29:58.000+11:00
diff --git a/source/MainFrm.cpp b/source/MainFrm.cpp
@@ -4826,101 +4826,55 @@ void CMainFrame::OnIdle(wxIdleEvent& event)
 		wxDateTime dt1 = wxDateTime::Now(),
 			dt2 = wxDateTime::UNow();
 #endif
-		// BEW 11Jun22, test for advancement of the phrasebox; first time, oldSN will be -1, and curSN will be
-		// a valid sequ num, for box location; but the two being unequal gives a correct bHasAdvanced value. 
-		// After that, oldSN will be set, and curSN will be at the later location - next pile or further on
-		// wherever a hole stops the box, and the test for equality will fail, and so an extra idleEvent will
-		// get requested below. But when the user has typed a new adaptation and hits Enter key, OnePass()
-		// will get that new value into the entry table of kbserver in a new row, as wxExecute() will have
-		// been called. The requested extra idleEvent will cause an immediate re-entry to line 4791 above,
-		// and we don't want that reentry to initiate a bogus (2nd) StoreText() done with the existing contents
-		// of the create_entry.dat file. So we have to do the following at this reentry situation:
-		// (a) compare oldSN and curSN - they will be identical because no advancement of the box has had
-		// a chance to happen. So that sets bHasAdvanced to FALSE.																								
-		// (b) the FALSE value for bHasAdvanced can then be used to do:
-		//  (i) Skip over the OnePass() call, and 
-		//  (ii) Skip over the request for an extra idleEvent - thereby killing the re-entrance issue
-		//  (iii) Halting the sequence of auto-insertions, thereby giving the user the opportunity to
-		//  type something at the next hole, and, in CallExecute() for do_create_entry.py or .exe, allows the
-		// program counter to progress in the normal way, to complete the create_entry block of the switch
-		// 
-		// BEW 3Aug22 added support to prevent call of OnePass() when the phrasebox is in a fixed location
-		// temporarily because a kbserver function (in CallExecute()) is being called and which does not
-		// involve relocating the phrasebox to some other location. The need for this was because
-		// do_pseudo_undelete.py was causing a bogus duplicate entry to be created in the entry table
-		// because OnIdle() fired and, without a test to make it's call of OnePass() be skipped, a duplicate
-		// entry resulted. Hmm, this logic addition blocks Enter from allowing movement forward - so too
-		// strong. Remove 
-		//bool bStationaryBox = StationaryPhraseBox();
 
 		// These next lines apply for the comments above prior to the 3Aug22 addition
-		bool bHasAdvanced = TRUE; // initialise
+		//bool bHasAdvanced = TRUE; // initialise  BEW commented out at Bill's suggestion, email 19Mar24 12:02pm
 		int oldSN = pApp->m_nOldSequNum;
 		int curSN = pApp->m_nActiveSequNum;
-		// whm 5Oct2023 Note: The following test is still too weak for proper operation of the
-		// phrasebox. For example, when an operation like "Remove A Retranslation" has just 
-		// completed, the phrasebox may not have advanced from its position and oldSN and currSN
-		// may still be the same making bHasAdvanced FALSE, and if so, the user cannot advance 
-		// the phrasebox by pressing Enter.
-		// IMO here in the OnIdle() handler is not a good place to try to make such as restriction
-		// against the OnePass() function getting executed whenever >m_bAutoInsert is TRUE.
-		// As a temporary solution, I'm inicluding the if (gpApp->m_bIsKBServerProject || gpApp->m_bIsGlossingKBServerProject)
-		// tests along with the curSN == oldSN test below to prevent the code here from interferring
-		// with the Enter key press functionality.
-		//if (curSN == oldSN)
-		if (curSN == oldSN && (gpApp->m_bIsKBServerProject || gpApp->m_bIsGlossingKBServerProject))
-		{
-			bHasAdvanced = FALSE;
-		}
-#if defined (_DEBUG)
-		wxLogDebug(_T("OnIdle() line %d, oldSN = %d , curSN = %d, bHasAdvanced = %d\n"), __LINE__,
-			oldSN, curSN, (int)bHasAdvanced);
-#endif
-		// Testing do_create_entry.py or .exe by running the app (in _DEBUG mode) resulted in a re-entrancy 
-		// loop which, if not stopped, would cause and infinite loop of calls of OnePass() until the 
-		// app crashes due to stack overflow. So I've prevented this using oldSN and curSN as above
-		//if ((bHasAdvanced == TRUE) && !bStationaryBox)
-		if (bHasAdvanced == TRUE)
-		{
-			CAdapt_ItApp* pApp = &wxGetApp(); pApp = pApp; // avoid warning in Release mode
-			CSourcePhrase* pSrcPhrase = NULL; pSrcPhrase = pSrcPhrase; // avoid warning in Release mode
+
+		bool bIsKBSvrAdaptProj = gpApp->m_bIsKBServerProject;
+		bool bIsKBSvGlossProj = gpApp->m_bIsGlossingKBServerProject;
+
+		// BEW 19Mar24 removed the constraining test here, which uses bHasAdvanced, at Bill's request
+		// and to retest to see if reentrancy problems arise from the today's changes
+		CAdapt_ItApp* pApp = &wxGetApp(); pApp = pApp; // avoid warning in Release mode
+		CSourcePhrase* pSrcPhrase = NULL; pSrcPhrase = pSrcPhrase; // avoid warning in Release mode
 #if defined(_DEBUG) //&& defined(FLAGS)
-			{	
-				if (pApp->m_pActivePile != NULL)
+		{	
+			if (pApp->m_pActivePile != NULL)
+			{
+				pSrcPhrase = pApp->m_pActivePile->GetSrcPhrase();
+				if (pSrcPhrase->m_nSequNumber >= 12)
 				{
-					pSrcPhrase = pApp->m_pActivePile->GetSrcPhrase();
-					if (pSrcPhrase->m_nSequNumber >= 12)
-					{
-						wxLogDebug(_T("\nOnIdle(), line %d, sn=%d, m_key= %s, m_bAbandonable %d, m_bRetainBoxContents %d, m_bUserTypedSomething %d, bHasAdvanced= [%d], m_bAutoInsert %d  OnePass() NEXT"),
-							__LINE__, pSrcPhrase->m_nSequNumber, pSrcPhrase->m_key.c_str(), (int)pApp->m_pTargetBox->m_bAbandonable, (int)pApp->m_pTargetBox->m_bRetainBoxContents,
-							(int)pApp->m_bUserTypedSomething, (int)bHasAdvanced, (int)pApp->m_bAutoInsert);
-					}
+					wxLogDebug(_T("\nOnIdle(), line %d, sn=%d, m_key= %s, m_bAbandonable %d, m_bRetainBoxContents %d, m_bUserTypedSomething %d, m_bAutoInsert %d, bIsKBSvrAdaptProj %d, bIsKBSvGlossProj %d, OnePass() NEXT"),
+						__LINE__, pSrcPhrase->m_nSequNumber, pSrcPhrase->m_key.c_str(), (int)pApp->m_pTargetBox->m_bAbandonable, (int)pApp->m_pTargetBox->m_bRetainBoxContents,
+						(int)pApp->m_bUserTypedSomething, (int)pApp->m_bAutoInsert, (int)bIsKBSvrAdaptProj, (int)bIsKBSvGlossProj);
 				}
 			}
+		}
 #endif
-			// Do the call, but skip this block if the phrasebox has not advanced
-			bSuccessfulInsertAndMove = pBox->OnePass(pView); // whm note: This is
+		// Do the call, but skip this block if the phrasebox has not advanced
+		bSuccessfulInsertAndMove = pBox->OnePass(pView); // whm note: This is
 											// the only place OnePass() is called
 #if defined (_DEBUG)
-			if (bSuccessfulInsertAndMove)
+		if (bSuccessfulInsertAndMove)
+		{
+			pSrcPhrase = pApp->m_pActivePile->GetSrcPhrase();
+			if (pSrcPhrase->m_nSequNumber >= 12)
 			{
-				pSrcPhrase = pApp->m_pActivePile->GetSrcPhrase();
-				if (pSrcPhrase->m_nSequNumber >= 12)
-				{
-					wxLogDebug(_T("%s::%s(), line %d, sn=%d, m_key= [%s], m_targetStr= [%s], m_bAutoInsert %d  OnePass() JUST HAPPENED"),
-						__FILE__, __FUNCTION__, __LINE__, pSrcPhrase->m_nSequNumber, pSrcPhrase->m_key.c_str(), pSrcPhrase->m_targetStr.c_str(),
-						 (int)pApp->m_bAutoInsert);
-				}
+				wxLogDebug(_T("OnIdle(), line %d, sn=%d, m_key= [%s], m_targetStr= [%s], m_bAutoInsert %d, bIsKBSvrAdaptProj %d, bIsKBSvGlossProj %d,  OnePass() JUST HAPPENED"),
+						__LINE__, pSrcPhrase->m_nSequNumber, pSrcPhrase->m_key.c_str(), pSrcPhrase->m_targetStr.c_str(),
+						(int)pApp->m_bAutoInsert, (int)pApp->m_bAutoInsert, (int)bIsKBSvrAdaptProj, (int)bIsKBSvGlossProj);
 			}
-#endif
 		}
+#endif
 
 		// don't slow things down unless we need to investigate
 #ifdef SHOW_ONEPASS_BENCHMARKS
-				dt1 = dt2;
-				dt2 = wxDateTime::UNow();
-				wxLogDebug(_T("********In OnIdle() OnePass() executed in %s ms"),
-					(dt2 - dt1).Format(_T("%l")).c_str());
+			dt1 = dt2;
+			dt2 = wxDateTime::UNow();
+			wxLogDebug(_T("********In OnIdle() OnePass() executed in %s ms"),
+				(dt2 - dt1).Format(_T("%l")).c_str());
 #endif
 
 		// whm added 20Nov10 reset the m_bIsGuess flag below. Can't do it in PlaceBox()
@@ -4930,53 +4884,16 @@ void CMainFrame::OnIdle(wxIdleEvent& event)
 		pApp->m_bIsGuess = FALSE;
 		if (bSuccessfulInsertAndMove)
 		{
-			// enable next iteration, but not when so-enabling would result in an infinite loop of reentrancy
-			if (bHasAdvanced)
-			{
-				// If StoreText has done a CallExecute() for the create_entry enum block, then
-				// the user must have made a new translation - which can only be done while the
-				// phrasebox was halted, and in that circumstance, immediately requesting a new
-				// idleEvent be posted, as here, would cause immediate re-entrancy and calling
-				// of OnPass() again wrongly, which would lead to an unending loop until the
-				// app crashs due to stack overflow. Disallowing RequestMore() when the box
-				// has not advanced, kills the reentrancy problem
 				event.RequestMore(); // Bill added
-			}
-			else
-			{
-				// BEW 11Jun22, the phrasebox has not advanced, so bring the
-				// box to a halt, but continue to enable OnIdle calls
-				pApp->m_bAutoInsert = FALSE;
-			}
 		}
 		else
 		{
-			// halt iterations, we did not make a match or could not move forward,
+			// halt iterations, we did not succeed or could not move forward,
 			// but continue to enable OnIdle calls
 			pApp->m_bAutoInsert = FALSE;
+			event.RequestMore(); // BEW added, 19Mar24
 		}
 
-		// BEW 28/jul23, when moving forward to pNewPile returns NULL, we've probably reached
-		// unexpected doc end, so perhaps returning here when bSuccessfulInsertAndMove is 
-		// FALSE might be helpful - as the calls further down will certainly fail in this circumstance
-		// ? No, I'm scared, maybe the bool can be false for other scenarios which don't destroy the app, I'll comment it out for a few levels
-		//if (!bSuccessfulInsertAndMove)
-		//{
-		//	return;
-		//}
-		/* #if defined(_DEBUG) && defined(FLAGS)
-		{
-			CAdapt_ItApp* pApp = &wxGetApp();
-			CSourcePhrase* pSrcPhrase = NULL;
-			if (pApp->m_pActivePile != NULL)
-			{
-				pSrcPhrase = pApp->m_pActivePile->GetSrcPhrase();
-				wxLogDebug(_T("\n%s::%s(), line %d, sn=%d, m_key= %s, m_bAbandonable %d, m_bRetainBoxContents %d, m_bUserTypedSomething %d, m_bBoxTextByCopyOnly %d, m_bAutoInsert %d"),
-					__FILE__, __FUNCTION__, __LINE__, pSrcPhrase->m_nSequNumber, pSrcPhrase->m_key.c_str(), (int)pApp->m_pTargetBox->m_bAbandonable, (int)pApp->m_pTargetBox->m_bRetainBoxContents,
-					(int)pApp->m_bUserTypedSomething, (int)pApp->m_pTargetBox->m_bBoxTextByCopyOnly, (int)pApp->m_bAutoInsert);
-			}
-		}
-#endif */
 	} // end of TRUE block for test: if (pApp->m_bAutoInsert)
     // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
     // whm added 10Jan2018 to support quick selection of a translation equivalent.
@@ -5055,7 +4972,6 @@ void CMainFrame::OnIdle(wxIdleEvent& event)
 //#endif
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
-
 	// BEW 2Dec2014 Alan Buseman's Guesser - support for hiding the GuesserUpdate() calls
 	// which need to be done pretty often -- and which block the GUI whether done synchronously
 	// as is done here, or asynchronously on a thread (due to mutexes blocking KB saves and
diff --git a/source/PhraseBox.cpp b/source/PhraseBox.cpp
@@ -3359,8 +3359,8 @@ bool CPhraseBox::LookAhead(CPile* pNewPile)
 
 // BEW 13Apr10, no changes needed for support of doc version 5
 // BEW 8July10, no changes needed for support of kbVersion 2
-// BEW 18Mar24, JumpForward() is called from within OnePass(), OnSysKeyUp(), OnKeyUp() and in the
-// OnOK() function of Preferences... for the Punctuation Correspondences Page.
+// BEW 18Mar24, JumpForward() is called from within OnePass() but only in transliteration mode, 
+// OnSysKeyUp(), OnKeyUp() and in the OnOK() function of Preferences... for the Punctuation Correspondences Page.
 // I needed to refactor JumpForward() because the auto-insert mode, which accepts Enter or Tab for 
 // jumping to next 'hole', did not have a call of MoveToNextPile() to cause the foward movement 
 // to happen. Fixed that. Now appears to work right. The error was discovered when I came to a 
@@ -3637,56 +3637,12 @@ void CPhraseBox::JumpForward(CAdapt_ItView* pView)
 		} // end of block for test for m_bSingleStep == TRUE
 		else // auto-inserting 
 		{
-
-		//*
-			// BEW 18Mar24, Bill is convinced that this block of code (to the //*/ line) is not needed.
-			// On his machine, which does not have a MariaDB installation, Enter works just fine for a
-			// merger followed by Enter keypress. On my machine (M90T), the box is paralysed - does not advance.
-			// I've made multiple tests, including nyindrou data, no kb sharing set up on project, MariaDB not
-			// running, and every time the box at Enter press, it does not move - whether project is a kb sharing one,
-			// or not, if this following block of code is commented out. So I'm keeping the code as is. It works.
-			// I can't afford to spend more time on the issue, especially as I've no realistic way to compare the
-			// workings of my computer versus Bill's.
-			
 			// cause auto-inserting using the OnIdle handler to commence
 			pApp->m_bAutoInsert = TRUE;
-			if (pApp->m_pTargetBox->m_bAbandonable)
-			{
-				pApp->m_pTargetBox->m_bAbandonable = FALSE; // yep, checking and resetting 
-					//FALSE here releaves the user of responsibility
-					// to make the content 'stick' by first doing something, eg. a click, in the box
-					// before using ENTER key or TAB key to cause an advance to a hole
-			}
-			// User has pressed the Enter key  (OnChar() calls JumpForward())
-			// BEW changed 9Apr12, to support discontinuous highlighting
-			// spans for auto-insertions...
-			// Since OnIdle() will call OnePass() and the latter will call
-			// MoveToNextPile(), and it is in MoveToNextPile() that CCell's
-			// m_bAutoInserted flag can get set TRUE, we only here need to ensure that the
-			// current location is a kick-off one, which we can do by clearing any earlier
-			// highlighting currently in effect
-			pLayout->ClearAutoInsertionsHighlighting();
-
-			pLayout->m_pDoc->ResetPartnerPileWidth(pApp->m_pActivePile->GetSrcPhrase());
-			int bSuccessful;
-			bSuccessful = MoveToNextPile(pApp->m_pActivePile);
-
-			pLayout->m_docEditOperationType = relocate_box_op;
-#ifdef _NEW_LAYOUT
-			pLayout->RecalcLayout(pApp->m_pSourcePhrases, keep_strips_keep_piles);
-#else
-			pLayout->RecalcLayout(pApp->m_pSourcePhrases, create_strips_keep_piles);
+#if defined(_DEBUG)
+			wxLogDebug(_T("10. JumpForward() line %d , pApp->m_bAutoInsert = TRUE has just been set"), __LINE__);
 #endif
-			pApp->m_pActivePile = pView->GetPile(pApp->m_nActiveSequNum);
-			wxASSERT(pApp->m_pActivePile != NULL);
-			CSourcePhrase* pSPhr = pApp->m_pActivePile->GetSrcPhrase();
-			pLayout->m_pDoc->ResetPartnerPileWidth(pSPhr);
-
-			pLayout->m_pCanvas->ScrollIntoView(pApp->m_nActiveSequNum);
 
-			pLayout->PlaceBox();
-			pView->Invalidate();
-		//*/
 		} // end of else block for test for m_bSingleStep == TRUE, i.e. autoinserting
 
 		// save the phrase box's text, in case user hits SHIFT+End to unmerge a
@@ -3889,11 +3845,11 @@ void CPhraseBox::JumpForward(CAdapt_ItView* pView)
 
 #ifdef _NEW_LAYOUT
 		#ifdef _FIND_DELAY
-			wxLogDebug(_T("10. Start of RecalcLayout in JumpForward"));
+			wxLogDebug(_T("11. Start of RecalcLayout in JumpForward"));
 		#endif
 		pLayout->RecalcLayout(pApp->m_pSourcePhrases, keep_strips_keep_piles);
 		#ifdef _FIND_DELAY
-			wxLogDebug(_T("11. End of RecalcLayout in JumpForward"));
+			wxLogDebug(_T("12. End of RecalcLayout in JumpForward"));
 		#endif
 #else
 		pLayout->RecalcLayout(pApp->m_pSourcePhrases, create_strips_keep_piles);
@@ -3906,9 +3862,9 @@ void CPhraseBox::JumpForward(CAdapt_ItView* pView)
 		pView->Invalidate();
 		pLayout->PlaceBox();
 	} // end Review mode (single src phrase move) block
-	#ifdef _FIND_DELAY
-		wxLogDebug(_T("12. End of JumpForward"));
-	#endif
+	//#ifdef _FIND_DELAY
+		wxLogDebug(_T("13. End of JumpForward(pView), returning now"));
+	//#endif
 }
 
 // This function is called for every character typed in phrase box (via OnChar() function 
@@ -6236,9 +6192,9 @@ void CPhraseBox::OnSysKeyUp(wxKeyEvent& event)
 
 bool CPhraseBox::OnePass(CAdapt_ItView *pView)
 {
-#ifdef _FIND_DELAY
-		wxLogDebug(_T("1. Start of OnePass"));
-#endif
+//#ifdef _FIND_DELAY
+		wxLogDebug(_T("1. Start of OnePass(pView)"));
+//#endif
 	CAdapt_ItApp* pApp = &wxGetApp();
 	wxASSERT(pApp != NULL);
     CAdapt_ItDoc* pDoc = pApp->GetDocument();
@@ -6338,18 +6294,18 @@ bool CPhraseBox::OnePass(CAdapt_ItView *pView)
 	}
 	else
 	{
-#ifdef _FIND_DELAY
-		wxLogDebug(_T("2. Before MoveToNextPile"));
-#endif
+//#ifdef _FIND_DELAY
+		wxLogDebug(_T("2. OnePass(), Before MoveToNextPile() is to be called"));
+//#endif
 		bSuccessful = MoveToNextPile(pApp->m_pActivePile);
 		// BEW 28Jul23, nah, too dangerous - there's "not successful" code blocks below which may be workable
 		//if (!bSuccessful) // BEW added 28Jul23
 		//{
 		//	return FALSE; // retuns to MainFrm.cpp				  
 		//}
-		#ifdef _FIND_DELAY
-			wxLogDebug(_T("3. After MoveToNextPile"));
-		#endif
+		//#ifdef _FIND_DELAY
+			wxLogDebug(_T("3. OnePass() After MoveToNextPile() has advanced phrasebox forward"));
+		//#endif
 		// If in vertical edit mode, and the phrasebox has come to a hole, then we
 		// want to halt OnePass() calls in OnIdle() so that the user gets the chance
 		// to adapt at the hole
@@ -6407,7 +6363,7 @@ bool CPhraseBox::OnePass(CAdapt_ItView *pView)
             // YES!! That works - the highlighting is now visible when the box has
             // disappeared and the end of doc message shows. Also, normal adapting still
             // works right despite this change, so that's a bug (or undesireable feature -
-            // namely, the loss of highlighting when the doc is reached by auto-inserting)
+            // namely, the loss of highlighting when the doc end is reached by auto-inserting)
             // now fixed.
 
 			pLayout->Redraw(); // bFirstClear is default TRUE
@@ -6489,9 +6445,9 @@ bool CPhraseBox::OnePass(CAdapt_ItView *pView)
 	pLayout->m_docEditOperationType = relocate_box_op;
 	pView->Invalidate(); // added 1Apr09, since we return at next line
 	pLayout->PlaceBox();
-	#ifdef _FIND_DELAY
-		wxLogDebug(_T("8. End of OnePass"));
-	#endif
+	//#ifdef _FIND_DELAY
+		wxLogDebug(_T("8. End of OnePass(pView)"));
+	//#endif
 
 	pApp->m_bUserDlgOrMessageRequested = FALSE;
 	pApp->m_bUserHitEnterOrTab = FALSE;
@@ -6841,11 +6797,17 @@ void CPhraseBox::OnKeyUp(wxKeyEvent& event)
 		// a placeholder, Choose Translation, Guesser dialog, and maybe others (but not all).
 		// Whether in Drafting or Reviewing mode, without the protection below, then an Enter
 		// or Tab key press would cause run-on the next hole, or next pile, respectively. 
-         
-        // Normal jump is okay to do
+#if defined (_DEBUG)
+		wxLogDebug(_T("OnKeyUp() in PhraseBox.cpp, line %d, JumpForward() called on next line, then return"), __LINE__);
+#endif
+        // Normal jump is okay to do - I restored the line pApp->m_bAutoInsert = TRUE; at the else block where
+		// it's not m_bSingleStep TRUE; we want OnIdle to trigger a call of OnePass() instead of doing something in 
+		// JumpForward(). This call will complete and set m_bAutoInsert before OnIdle has a chance to trigger an
+		// idle event and that's where (via it's handler) OnePass() gets the chance to be called, responding to
+		// the user's ENTER or TAB keypress
         JumpForward(pView);
         return;
-    }
+    } // end of TRUE block for test: if keycode is TAB or Enter etc.
 
     // version 1.4.2 and onwards, we want a right or left arrow used to remove the
 	// phrasebox's selection to be considered a typed character, so that if a subsequent
