Recent

Author Topic: Retain anchored desktop configuration  (Read 7454 times)

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Retain anchored desktop configuration
« on: June 28, 2017, 11:15:43 am »
Attention jc99. I applied your patch in r55410.
Please test everybody.

The patch was in the "Lazarus Release Candidate 2 of 1.8" thread.
I wanted to create this new topic because the change will not go to Lazarus 1.8. It must be tested well in trunk. The fixes_1_8 branch is hopefully stabilizing and gets only crucial bug fixes.
Note: a recommended way to provide patches is still through the bug tracker.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

Ondrej Pokorny

  • Full Member
  • ***
  • Posts: 220
Re: Retain anchored desktop configuration
« Reply #1 on: June 28, 2017, 09:40:21 pm »
I don't like the code. It looks more like a hack.

A nice object-oriented approach would be desirable.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Re: Retain anchored desktop configuration
« Reply #2 on: June 30, 2017, 11:37:39 am »
I don't like the code. It looks more like a hack.
A nice object-oriented approach would be desirable.
Maybe jc99 could improve it, or then you. (?)
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

jc99

  • Hero Member
  • *****
  • Posts: 553
    • My private Site
Re: Retain anchored desktop configuration
« Reply #3 on: June 30, 2017, 04:18:28 pm »
I don't like the code. It looks more like a hack.
A nice object-oriented approach would be desirable.
Maybe jc99 could improve it, or then you. (?)
It's a hack, it kicks in when TDesktopOpt refuses to store the Layout.
TDesktopOpt on the other hand seems to have the ability to store docked Layouts at least it has the isDocked - Property
But to improve something, then be more specific. To me it was the smallest possible solution, with least side-effects. 

« Last Edit: June 30, 2017, 04:22:15 pm by jc99 »
OS: Win XP x64, Win 7, Win 7 x64, Win 10, Win 10 x64, Suse Linux 13.2
Laz: 1.4 - 1.8.4, 2.0
https://github.com/joecare99/public
'~|    /''
,_|oe \_,are
If you want to do something for the environment: Twitter: #reduceCO2 or
https://www.betterplace.me/klimawandel-stoppen-co-ueber-preis-reduzieren

Ondrej Pokorny

  • Full Member
  • ***
  • Posts: 220
Re: Retain anchored desktop configuration
« Reply #4 on: June 30, 2017, 10:50:34 pm »
But to improve something, then be more specific.

I'd create a TUnsupportedDesktopOpt class that would be used instead of TDesktopOpt:

Code: [Select]
procedure TDesktopOptList.AddFromCfg(Path: String);
var
  dsk: TDesktopOpt;
  dskClass: TDesktopOptClass;
  dskName, dskDockMaster: String;
begin
  dskName := FXMLCfg.GetValue(Path+'Name', 'default');
  dskDockMaster := FXMLCfg.GetValue(Path+'DockMaster', '');

  if (IndexOf(dskName) >= 0) then
    Exit;

  if EnvironmentOptions.DesktopCanBeLoaded(dskDockMaster) then
    dskClass := TDesktopOpt
  else
    dskClass := TUnsupportedDesktopOpt;

  dsk := dskClass.Create(dskName, dskDockMaster<>'');
  dsk.SetConfig(FXMLCfg, FConfigStore);
  dsk.Load(Path);
  Add(dsk);
end;

The TUnsupportedDesktopOpt should override GetCompatible and always return False. It should store the whole XML node tree into memory and save it back.

A decent parent structure has to be created, obviously (maybe TAbstractDesktopOpt, TDesktopOpt = class(TAbstractDesktopOpt), TUnsupportedDesktopOpt = class(TAbstractDesktopOpt) or similar).
« Last Edit: June 30, 2017, 10:57:27 pm by Ondrej Pokorny »

Ondrej Pokorny

  • Full Member
  • ***
  • Posts: 220
Re: Retain anchored desktop configuration
« Reply #5 on: June 30, 2017, 10:56:13 pm »
I reverted your patch. I don't want any hacks in Lazarus sources unless absolutely necessary. I created a mantis report as well: https://mantis.freepascal.org/view.php?id=32083

jc99

  • Hero Member
  • *****
  • Posts: 553
    • My private Site
Re: Retain anchored desktop configuration
« Reply #6 on: June 30, 2017, 11:01:46 pm »
But to improve something, then be more specific.

I'd create a TUnsupportedDesktopOpt class that would be used instead of TDesktopOpt:

Code: [Select]
procedure TDesktopOptList.AddFromCfg(Path: String);
var
  dsk: TDesktopOpt;
  dskClass: TDesktopOptClass;
  dskName, dskDockMaster: String;
begin
  dskName := FXMLCfg.GetValue(Path+'Name', 'default');
  dskDockMaster := FXMLCfg.GetValue(Path+'DockMaster', '');

  if (IndexOf(dskName) >= 0) then
    Exit;

  if EnvironmentOptions.DesktopCanBeLoaded(dskDockMaster) then
    dskClass := TDesktopOpt
  else
    dskClass := TUnsupportedDesktopOpt;

  dsk := dskClass.Create(dskName, dskDockMaster<>'');
  dsk.SetConfig(FXMLCfg, FConfigStore);
  dsk.Load(Path);
  Add(dsk);
end;

The TUnsupportedDesktopOpt should override GetCompatible and always return False. It should store the whole XML node tree into memory and save it back.

A decent parent structure has to be created, obviously (maybe TAbstractDesktopOpt, TDesktopOpt, TUnsupportedDesktopOpt or similar).

That was the first thing I had in mind too, I didn't know you'd support such a bigger change. So I went for the small solution. Is it OK to publish the patch here, or should I (or have you already) create a bug-tracker-event ?
[edit]Ok, forget  the last sentence ...
OS: Win XP x64, Win 7, Win 7 x64, Win 10, Win 10 x64, Suse Linux 13.2
Laz: 1.4 - 1.8.4, 2.0
https://github.com/joecare99/public
'~|    /''
,_|oe \_,are
If you want to do something for the environment: Twitter: #reduceCO2 or
https://www.betterplace.me/klimawandel-stoppen-co-ueber-preis-reduzieren

jc99

  • Hero Member
  • *****
  • Posts: 553
    • My private Site
Re: Retain anchored desktop configuration
« Reply #7 on: June 30, 2017, 11:12:01 pm »
Is there an official test-environment for lazarus, to test new/changed components ? If no then at least I got one for the EnvironmentOpts system.
https://github.com/joecare99/Testlaz
« Last Edit: June 30, 2017, 11:34:23 pm by jc99 »
OS: Win XP x64, Win 7, Win 7 x64, Win 10, Win 10 x64, Suse Linux 13.2
Laz: 1.4 - 1.8.4, 2.0
https://github.com/joecare99/public
'~|    /''
,_|oe \_,are
If you want to do something for the environment: Twitter: #reduceCO2 or
https://www.betterplace.me/klimawandel-stoppen-co-ueber-preis-reduzieren

jc99

  • Hero Member
  • *****
  • Posts: 553
    • My private Site
Re: !! Help !! Retain anchored desktop configuration
« Reply #8 on: July 01, 2017, 09:20:19 pm »
Hi I need help !
I try to store the part of the configuration belonging to the Unsupported DesktopOpt into a separate XML-Document but could not get it to work maybe someone could help.
at the moment :
Load & Save procedure
Code: Pascal  [Select][+][-]
  1. type
  2.   { TUnsupportedDesktopOpt }
  3.  
  4.   TUnsupportedDesktopOpt = Class(TAbstractDesktopOpt)
  5.   private
  6.     FRetainXMLData:TDOMDocument; {This or some other appropriate structure}
  7.   public
  8.     destructor Destroy; override;
  9.     procedure Load(Path: String); override;
  10.     procedure Save(Path: String); override;
  11.   end;
  12.  
  13. //[...]
  14. procedure TUnsupportedDesktopOpt.Load(Path: String);
  15. var
  16.   lPnode, lChldNode: TDOMNode;
  17.   i: Integer;
  18. begin
  19.   FreeAndNil(FRetainXMLData);
  20.   FRetainXMLData:=TDOMDocument.Create;
  21.   lPnode := FXMLCfg.FindNode(Path,false);
  22.   for i := 0 to lPnode.ChildNodes.Count-1 do
  23.     begin
  24.       lChldNode := lPnode.ChildNodes[i].CloneNode(true);
  25.   [/i]    FRetainXMLData.ImportNode(lChldNode,true); // to set the owner of the cloned node to the Document
  26.       FRetainXMLData.AppendChild(lChldNode); // to actually append the node
  27.     end;
  28. end;
  29.  
  30. procedure TUnsupportedDesktopOpt.Save(Path: String);
  31. var
  32.   i: Integer;
  33.   lChldNode: TDOMNode;
  34. begin
  35.   if assigned(FRetainXMLData) and assigned(FXMLCfg.FindNode(path,false)) then
  36.   for i := 0 to FRetainXMLData.ChildNodes.Count-1 do
  37.     begin
  38.       lChldNode := FRetainXMLData.ChildNodes.CloneNode(true); // Make a copy of the node
  39.       FXMLCfg.Document.ImportNode(lChldNode,true);  // Import the copied nodes to the Configuration-document
  40.       FXMLCfg.FindNode(path,false).AppendChild(lChldNode); // append the nodes at the right place
  41.     end;
  42. end;
  43.  

Test-Environment at:
https://github.com/joecare99/Testlaz

PS.: of cause there ist the ("by foot") method, to copy the nodes "manually" -step by step, but I am interested in a more "slinky" method.
[edit] attached the full change as .patch
« Last Edit: July 01, 2017, 09:31:31 pm by jc99 »
OS: Win XP x64, Win 7, Win 7 x64, Win 10, Win 10 x64, Suse Linux 13.2
Laz: 1.4 - 1.8.4, 2.0
https://github.com/joecare99/public
'~|    /''
,_|oe \_,are
If you want to do something for the environment: Twitter: #reduceCO2 or
https://www.betterplace.me/klimawandel-stoppen-co-ueber-preis-reduzieren

jc99

  • Hero Member
  • *****
  • Posts: 553
    • My private Site
Re: [SOLVED] Retain anchored desktop configuration
« Reply #9 on: July 02, 2017, 03:18:33 am »
Got it ...
[final] patch uploaded
see mantis/bugtracker ...
OS: Win XP x64, Win 7, Win 7 x64, Win 10, Win 10 x64, Suse Linux 13.2
Laz: 1.4 - 1.8.4, 2.0
https://github.com/joecare99/public
'~|    /''
,_|oe \_,are
If you want to do something for the environment: Twitter: #reduceCO2 or
https://www.betterplace.me/klimawandel-stoppen-co-ueber-preis-reduzieren

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Re: !! Help !! Retain anchored desktop configuration
« Reply #10 on: July 02, 2017, 11:23:36 am »
Hi I need help !
Good you figured it out.
In general you can ask detailed technical questions about Lazarus code in the mailing list. All developers follow it, including Mattias who knows best the XML classes.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

jc99

  • Hero Member
  • *****
  • Posts: 553
    • My private Site
Re: Retain anchored desktop configuration
« Reply #11 on: July 02, 2017, 01:09:57 pm »
@OP:
You closeed the issue a little to soon, i just wanted to say:"It works in trunk", but not with 1.8 lcl, but in trunk all is fine, sorry for the fuzz.
So looking forward to the 1.10 or 2.0 release when it's included (hopefully)

About test-environment ? Is it a good idea ?     

[Edit]
I just tested the new version. Is there a reason, why you can/have to be able to select a incompatible destop as debugging-DT to a compatible one ?
My understanding of Unsupported is "unsupported" meaning not supported so not to be selectable a debug and also handled as less as possible.
therefore in Debug-Dropdown are only supported desktops. And was disabled on unsupported DTs. Maybe some other actions should be deactivated too,
the "set debug-Desktop" and "set active" -actions for instance.

And i missed a small one:
env... line 1083:
Code: Pascal  [Select][+][-]
  1.         .ReplaceChild(lChldNode, lParentNode)


   
« Last Edit: July 02, 2017, 01:53:30 pm by jc99 »
OS: Win XP x64, Win 7, Win 7 x64, Win 10, Win 10 x64, Suse Linux 13.2
Laz: 1.4 - 1.8.4, 2.0
https://github.com/joecare99/public
'~|    /''
,_|oe \_,are
If you want to do something for the environment: Twitter: #reduceCO2 or
https://www.betterplace.me/klimawandel-stoppen-co-ueber-preis-reduzieren

Ondrej Pokorny

  • Full Member
  • ***
  • Posts: 220
Re: Retain anchored desktop configuration
« Reply #12 on: July 02, 2017, 01:22:22 pm »
I simplified the code and it should be compatible with 1.8 now.

I'll check the test environment later, thanks.

 

TinyPortal © 2005-2018