--===============4100698852082590973== Content-Type: multipart/alternative; boundary="===============4810168692839309773==" --===============4810168692839309773== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit > On June 30, 2014, 4:28 p.m., Michal Humpula wrote: > > @Dominik: is it still possible to add new methods to the interface? > > > > To bad, when drafting the splitView I didn't think of more general abstraction like group of views. Methods like "splitView" and "closeViewSpace" seems to enable only one way to implement grouping of views. Oh, well, let's hope there will be KF6 one day:o) > > Christoph Cullmann wrote: > We can add still functions, as they are non-virtual, as we use the "ugly" but BC hack of internal invokeMethod. I wasn't sure about this either. That's a relief :) > On June 30, 2014, 4:28 p.m., Michal Humpula wrote: > > src/include/ktexteditor/mainwindow.h, line 155 > > > > > > I would recommend passing KTextEditor::View here, to let app know, which ViewSpace is to be closed. Worst case scenario, the implementation would have to call activeView() before calling closeViewSpace. > > Miquel Sabaté Solà wrote: > Yes, that was my initial take on this :) But then I realized that I only needed to close the current view. If we pass a view, then in the application you have to pick the viewspace that contains the given view. This is quite easy to do for Kate. > > I'll wait for more opinions on this ;) > > Christoph Cullmann wrote: > The question is: should not "closeView" be fixed instead. Somehow, the current behavior looks like a bug, I would think that it shall close the viewspace, if the last view got closed there (and no other document is around). > > If not, I would vote for calling the new function: > > "closeSplitView", which matches the splitView operation, which creates it. I also thought about this but I think it's ok to keep "closeView" as it is. If we say that "closeView" closes the viewspace, it's going to be messy, because there are multiple applications using the katepart with a different understanding of "group of views". So, I think that we should keep "closeView" as it is (closing a single view), and let the "let's close a group of views" function handle groups of views (so every application will have a different take on this). Thus, to me, it's a semantic difference that we should keep :) In regards to the name, I called it "closeViewSpace" because it's how it's called in Kate. I agree, I should rename it to "closeSplitView". But, should it take a "view" parameter as Michal said? I honestly don't know :) - Miquel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119028/#review61294 ----------------------------------------------------------- On June 30, 2014, 11:51 a.m., Miquel Sabaté Solà wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119028/ > ----------------------------------------------------------- > > (Updated June 30, 2014, 11:51 a.m.) > > > Review request for Kate. > > > Repository: ktexteditor > > > Description > ------- > > This is a method that I needed in order to implement the "close" command in vi-mode. The close command in vim closes the current "window". This concept of "window" is basically a splitted view. That's why other commands in vi-mode such as "split" and "vsplit" have been implemented by just splitting the current view. However, it's not possible to close a splitted view. The only thing that is close is the "closeView" method, but it's not really what we want, since calling this function will result in the image that I have uploaded. Therefore, before this patch, the KatePart allowed the creation of splitted views, but it didn't allow the deletion of splitted views (and that's inconsistent imho). > > With this function, now we can close splitted views as expected. In Kate (frameworks branch), the implementation of this function is trivial because the KateMainWindow class has access to the KateViewManager class (through m_viewManager). > > > Diffs > ----- > > src/include/ktexteditor/mainwindow.h fda80a2 > src/utils/mainwindow.cpp 1231f9a > > Diff: https://git.reviewboard.kde.org/r/119028/diff/ > > > Testing > ------- > > Manual testing. > > > File Attachments > ---------------- > > closeView in action > https://git.reviewboard.kde.org/media/uploaded/files/2014/06/30/fa7ce216-cc21-4647-8f72-f663374623c4__kate.png > > > Thanks, > > Miquel Sabaté Solà > > --===============4810168692839309773== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119028/

On June 30th, 2014, 4:28 p.m. UTC, Michal Humpula wrote:

@Dominik: is it still possible to add new methods to the interface?

To bad, when drafting the splitView I didn't think of more general abstraction like group of views. Methods like "splitView" and "closeViewSpace" seems to enable only one way to implement grouping of views. Oh, well, let's hope there will be KF6 one day:o)

On June 30th, 2014, 6:38 p.m. UTC, Christoph Cullmann wrote:

We can add still functions, as they are non-virtual, as we use the "ugly" but BC hack of internal invokeMethod.
I wasn't sure about this either. That's a relief :)

On June 30th, 2014, 4:28 p.m. UTC, Michal Humpula wrote:

src/include/ktexteditor/mainwindow.h (Diff revision 1) 
public:
 

    155
    
    bool closeViewSpace();

  


 




  
I would recommend passing KTextEditor::View here, to let app know, which ViewSpace is to be closed. Worst case scenario, the implementation would have to call activeView() before calling closeViewSpace.

 




 
On June 30th, 2014, 4:43 p.m. UTC, Miquel Sabaté Solà wrote:

 

  
Yes, that was my initial take on this :) But then I realized that I only needed to close the current view. If we pass a view, then in the application you have to pick the viewspace that contains the given view. This is quite easy to do for Kate.

I'll wait for more opinions on this ;)

 






 
On June 30th, 2014, 6:39 p.m. UTC, Christoph Cullmann wrote:

 

  
The question is: should not "closeView" be fixed instead. Somehow, the current behavior looks like a bug, I would think that it shall close the viewspace, if the last view got closed there (and no other document is around).

If not, I would vote for calling the new function:

"closeSplitView", which matches the splitView operation, which creates it.

 











I also thought about this but I think it's ok to keep "closeView" as it is. If we say that "closeView" closes the viewspace, it's going to be messy, because there are multiple applications using the katepart with a different understanding of "group of views". So, I think that we should keep "closeView" as it is (closing a single view), and let the "let's close a group of views" function handle groups of views (so every application will have a different take on this). Thus, to me, it's a semantic difference that we should keep :)

In regards to the name, I called it "closeViewSpace" because it's how it's called in Kate. I agree, I should rename it to "closeSplitView". But, should it take a "view" parameter as Michal said? I honestly don't know :)








- Miquel






On June 30th, 2014, 11:51 a.m. UTC, Miquel Sabaté Solà wrote:











 

  


Review request for Kate.


By Miquel Sabaté Solà.




Updated June 30, 2014, 11:51 a.m.












 Repository: 
ktexteditor





Description 

 

 

  
   
This is a method that I needed in order to implement the "close" command in vi-mode. The close command in vim closes the current "window". This concept of "window" is basically a splitted view. That's why other commands in vi-mode such as "split" and "vsplit" have been implemented by just splitting the current view. However, it's not possible to close a splitted view. The only thing that is close is the "closeView" method, but it's not really what we want, since calling this function will result in the image that I have uploaded. Therefore, before this patch, the KatePart allowed the creation of splitted views, but it didn't allow the deletion of splitted views (and that's inconsistent imho).

With this function, now we can close splitted views as expected. In Kate (frameworks branch), the implementation of this function is trivial because the KateMainWindow class has access to the KateViewManager class (through m_viewManager).

  		
 






Testing 



 

  
   
Manual testing.

  		
 






Diffs 


    

 
  • src/include/ktexteditor/mainwindow.h (fda80a2)
    • 

 
  • src/utils/mainwindow.cpp (1231f9a)
    • 





View Diff





File Attachments 








  		
 











  

 



--===============4810168692839309773==--

--===============4100698852082590973==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel

--===============4100698852082590973==--