All Projects FPC fpcprojects fpprofilerfpGUI Lazarus Packages PatchesLazarus CCR Mantis pas2js

View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0038178LazarusIDEpublic2020-12-07 08:052020-12-08 20:35
ReporterOkobaPatino Assigned ToJuha Manninen  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version2.1 (SVN) 
Summary0038178: JCF set the cursor at the file beginning after each format
DescriptionWith a clean trunk install, after formatting the file, the cursor will be set to first of the unit or project.
TagsNo tags attached.
Fixed in Revisionr64185
LazTarget-
Widgetset
Attached Files

Relationships

related to 0038081 closedJuha Manninen [patch] Jedi code format improvement. Added support for Threadvar in classes and records. 

Activities

OkobaPatino

2020-12-07 08:09

reporter   ~0127392

It is working correctly in the recent version of Lazarus, I checked a version from December 1, and it works fine. Perhaps one of the changes in the past month broke it.

Juha Manninen

2020-12-07 10:11

developer   ~0127396

December 1 was less than a week ago. There are no commits for JCF since then. The last one is r64166 from November 30 :
  Jedi Code Format: Fix access violation when parsing not closed IFDEF. Issue 0038146, patch from Domingo Galmés.
Please check again.
FYI: When looking for a guilty revision from a large number of commits, bisecting helps :
 https://wiki.freepascal.org/How_do_I_create_a_bug_report#Regression_caused_by_a_certain_revision
It is basically a binary search algorithm over the revision history. Git has local history + a "git bisect" command which helps a lot.

OkobaPatino

2020-12-07 10:52

reporter   ~0127400

Sorry, I meant November. Yes probably @DomingoGP knows about this.
I will try to find the exact commit, thanks for the help.

OkobaPatino

2020-12-07 11:28

reporter   ~0127403

I think I found the guilty one.
SVN: trunk@64136 4005530d-fff6-0310-9dd1-cebe43e6787f
Git: 4cf95127a17a5dad45fe83f172dbb20da3958f03

Domingo Galmés

2020-12-07 11:47

reporter   ~0127405

This patch keeps the cursor position ( Same line, same column), not same text, the JCF always can delete or add lines after formating.
JCD_keep_cursor_position.patch (1,103 bytes)    
From 3d5dd97f6bc50b19e99b5713ca51586401b49bad Mon Sep 17 00:00:00 2001
From: DomingoGP <dgalmesp@gmail.com>
Date: Mon, 7 Dec 2020 11:42:22 +0100
Subject: [PATCH] Keeps cursor position when formatting all unit.

---
 components/jcf2/ReadWrite/EditorConverter.pas | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/components/jcf2/ReadWrite/EditorConverter.pas b/components/jcf2/ReadWrite/EditorConverter.pas
index c02dd231a1..cd612aaa1b 100644
--- a/components/jcf2/ReadWrite/EditorConverter.pas
+++ b/components/jcf2/ReadWrite/EditorConverter.pas
@@ -136,15 +136,19 @@ begin
 end;
 
 procedure TEditorConverter.WriteToIDE(const pcUnit: TSourceEditorInterface; const psText: string);
+var
+  lLogicalCaretXY:TPoint;
 begin
   if pcUnit = nil then
     exit;
   if psText <> fcConverter.InputCode then
   begin
     try
+      lLogicalCaretXY:=pcUnit.CursorTextXY;
       pcUnit.BeginUpdate;
       pcUnit.BeginUndoBlock;
       pcUnit.Lines.Text := psText;
+      pcUnit.CursorTextXY:=lLogicalCaretXY;
       pcUnit.Modified := True;
     finally
       pcUnit.EndUndoBlock;  
-- 
2.29.1.windows.1
JCD_keep_cursor_position.patch (1,103 bytes)   

OkobaPatino

2020-12-07 12:21

reporter   ~0127407

Last edited: 2020-12-07 12:22

View 2 revisions

 @DomingoGP Does this fix the undo issue? I just found out that this issue clears the undo history, so you can not undo after formatting code.

Juha Manninen

2020-12-07 12:44

developer   ~0127409

Last edited: 2020-12-07 12:45

View 2 revisions

 > SVN: trunk@64136 4005530d-fff6-0310-9dd1-cebe43e6787f
Thanks. I added the related issue. BTW, I don't even know what the long number with '-' means. It comes from SVN anyway, I also see it.

> Git: 4cf95127a17a5dad45fe83f172dbb20da3958f03
The Git SHA1 ID is not useful here because it depends on how you got the revisions into Git. For example I use git-svn link and have different IDs. One change in the history changes all the IDs.
FPC and Lazarus may switch to Git master later. Then the SHA1 ID matters.

> I just found out that this issue clears the undo history
Do you mean 0038081 + r64136 clears the undo history? How could it? It only affects JCF code but the undo history is built in Lazarus IDE+editor.

OkobaPatino

2020-12-07 12:49

reporter   ~0127412

I've got that ID from Git bisect. Should I share another one in the future?
I do not know how that causes undo, but I tried to write code, format, and then hitting Ctrl+Z will not do anything.
I reverted that commit, and they work fine, both undo and cursor.

Juha Manninen

2020-12-07 13:56

developer   ~0127415

> I've got that ID from Git bisect. Should I share another one in the future?
No, SVN revision is enough. Sequential revision numbers are a benefit of Subversion. Projects using Git as a master don't have it.

Domingo Galmés

2020-12-07 14:25

reporter   ~0127418

The problem is in the line
       pcUnit.Lines.Text := psText; // removes undo history.

The workarround is using replacetext instead of Lines.Text.

This patch includes the previos that preserves the cursor position.

The selection of all text is ugly but I don't know a better way for do it.

in ReadWrite/EditorConverter.pas
procedure TEditorConverter.WriteToIDE(const pcUnit: TSourceEditorInterface; const psText: string);
var
  lLogicalCaretXY:TPoint;
  lStart,lEnd:TPoint;
begin
  if pcUnit = nil then
    exit;
  if psText <> fcConverter.InputCode then
  begin
    try
      lLogicalCaretXY:=pcUnit.CursorTextXY;
      pcUnit.BeginUpdate;
      pcUnit.BeginUndoBlock;
      lStart.X:=0; //select all text.
      lStart.Y:=0;
      lEnd.X:=0;
      if pcUnit.LineCount>0 then
        lEnd.X:=length(pcUnit.Lines[pcUnit.LineCount-1])+1;
      lEnd.Y:=pcUnit.LineCount;
      //pcUnit.Lines.Text := psText; // removes undo history.
      pcUnit.ReplaceText(lStart,lEnd,psText);
      pcUnit.CursorTextXY:=lLogicalCaretXY;
      pcUnit.Modified := True;
    finally
      pcUnit.EndUndoBlock;
      pcUnit.EndUpdate;
    end;
  end;
end;
JCF_2_preserve_undo_history.patch (2,479 bytes)    
From 3d5dd97f6bc50b19e99b5713ca51586401b49bad Mon Sep 17 00:00:00 2001
From: DomingoGP <dgalmesp@gmail.com>
Date: Mon, 7 Dec 2020 11:42:22 +0100
Subject: [PATCH 1/2] Keeps cursor position when formatting all unit.

---
 components/jcf2/ReadWrite/EditorConverter.pas | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/components/jcf2/ReadWrite/EditorConverter.pas b/components/jcf2/ReadWrite/EditorConverter.pas
index c02dd231a1..cd612aaa1b 100644
--- a/components/jcf2/ReadWrite/EditorConverter.pas
+++ b/components/jcf2/ReadWrite/EditorConverter.pas
@@ -136,15 +136,19 @@ begin
 end;
 
 procedure TEditorConverter.WriteToIDE(const pcUnit: TSourceEditorInterface; const psText: string);
+var
+  lLogicalCaretXY:TPoint;
 begin
   if pcUnit = nil then
     exit;
   if psText <> fcConverter.InputCode then
   begin
     try
+      lLogicalCaretXY:=pcUnit.CursorTextXY;
       pcUnit.BeginUpdate;
       pcUnit.BeginUndoBlock;
       pcUnit.Lines.Text := psText;
+      pcUnit.CursorTextXY:=lLogicalCaretXY;
       pcUnit.Modified := True;
     finally
       pcUnit.EndUndoBlock;  
-- 
2.29.1.windows.1


From 943151aa6f65fe8d00e9c25f84d7b213e1a0ded3 Mon Sep 17 00:00:00 2001
From: DomingoGP <dgalmesp@gmail.com>
Date: Mon, 7 Dec 2020 14:12:46 +0100
Subject: [PATCH 2/2] Preserves undo history

---
 components/jcf2/ReadWrite/EditorConverter.pas | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/components/jcf2/ReadWrite/EditorConverter.pas b/components/jcf2/ReadWrite/EditorConverter.pas
index cd612aaa1b..571ae84595 100644
--- a/components/jcf2/ReadWrite/EditorConverter.pas
+++ b/components/jcf2/ReadWrite/EditorConverter.pas
@@ -138,6 +138,7 @@ end;
 procedure TEditorConverter.WriteToIDE(const pcUnit: TSourceEditorInterface; const psText: string);
 var
   lLogicalCaretXY:TPoint;
+  lStart,lEnd:TPoint;
 begin
   if pcUnit = nil then
     exit;
@@ -147,7 +148,14 @@ begin
       lLogicalCaretXY:=pcUnit.CursorTextXY;
       pcUnit.BeginUpdate;
       pcUnit.BeginUndoBlock;
-      pcUnit.Lines.Text := psText;
+      lStart.X:=0;  //select all text.
+      lStart.Y:=0;
+      lEnd.X:=0;
+      if pcUnit.LineCount>0 then
+        lEnd.X:=length(pcUnit.Lines[pcUnit.LineCount-1])+1;
+      lEnd.Y:=pcUnit.LineCount;
+      //pcUnit.Lines.Text := psText;    // removes undo history.
+      pcUnit.ReplaceText(lStart,lEnd,psText);
       pcUnit.CursorTextXY:=lLogicalCaretXY;
       pcUnit.Modified := True;
     finally
-- 
2.29.1.windows.1
JCF_2_preserve_undo_history.patch (2,479 bytes)   

OkobaPatino

2020-12-07 14:59

reporter   ~0127421

@DomingoGP Can I ask why that happens? As it seems, you made the ThreadVar patch that removes the undo history. How was it working before that patch?

Domingo Galmés

2020-12-07 15:13

reporter   ~0127423

@OkobaPatino
Well as I said in the patch 0038081
"I have encountered a problem formatting the whole document, JCF insert blank lines in random position when tries to synchonize the editor text with the already formatted text. The old code tries to keep the original lines that have not changed, but it is enough for JCF to insert or delete a blank line for the synchronization not to work. I have changed the code and now it replaces all the code in the editor."

The patch 0038081 also tries to resolve de formatting of the whole document, I admit that it would have been better to send two separate patches. Sorry.

OkobaPatino

2020-12-07 15:57

reporter   ~0127425

Thanks for the explanation. So there are three topics if I understood correctly:
1- Threadvar, fixed by your last patch.
2- Inserted blank lines, fixed by your last patch.
3- Cursor positions and Undo, caused by the 0000002.
Maybe it is better to revert the 0000002 as it seems to replace the whole code or select all of it, and perhaps it is not a very good idea?

Domingo Galmés

2020-12-07 16:56

reporter   ~0127426

I think the better is apply the JCF_2_preserve_undo_history.patch to the trunk without reverting the patch of threadvar.
This solves all the problems with undo and cursor position.

OkobaPatino

2020-12-08 15:32

reporter   ~0127459

@JuhaManninen, Can you please apply this patch? JCF is broken right now in Trunk.

Juha Manninen

2020-12-08 20:35

developer   ~0127464

Applied, thanks.

Issue History

Date Modified Username Field Change
2020-12-07 08:05 OkobaPatino New Issue
2020-12-07 08:09 OkobaPatino Note Added: 0127392
2020-12-07 10:11 Juha Manninen Note Added: 0127396
2020-12-07 10:52 OkobaPatino Note Added: 0127400
2020-12-07 11:28 OkobaPatino Note Added: 0127403
2020-12-07 11:47 Domingo Galmés Note Added: 0127405
2020-12-07 11:47 Domingo Galmés File Added: JCD_keep_cursor_position.patch
2020-12-07 12:21 OkobaPatino Note Added: 0127407
2020-12-07 12:22 OkobaPatino Note Edited: 0127407 View Revisions
2020-12-07 12:30 Juha Manninen Relationship added related to 0038081
2020-12-07 12:30 Juha Manninen Assigned To => Juha Manninen
2020-12-07 12:30 Juha Manninen Status new => assigned
2020-12-07 12:44 Juha Manninen Note Added: 0127409
2020-12-07 12:45 Juha Manninen Note Edited: 0127409 View Revisions
2020-12-07 12:49 OkobaPatino Note Added: 0127412
2020-12-07 13:56 Juha Manninen Note Added: 0127415
2020-12-07 14:25 Domingo Galmés Note Added: 0127418
2020-12-07 14:25 Domingo Galmés File Added: JCF_2_preserve_undo_history.patch
2020-12-07 14:59 OkobaPatino Note Added: 0127421
2020-12-07 15:13 Domingo Galmés Note Added: 0127423
2020-12-07 15:57 OkobaPatino Note Added: 0127425
2020-12-07 16:56 Domingo Galmés Note Added: 0127426
2020-12-08 15:32 OkobaPatino Note Added: 0127459
2020-12-08 20:35 Juha Manninen Status assigned => resolved
2020-12-08 20:35 Juha Manninen Resolution open => fixed
2020-12-08 20:35 Juha Manninen Fixed in Revision => r64185
2020-12-08 20:35 Juha Manninen LazTarget => -
2020-12-08 20:35 Juha Manninen Note Added: 0127464