This revision was automatically updated to reflect the committed changes.
Closed by commit R39:c9cb3e9761f3: Expose additional internal View's
functionality to the public API (authored by jsalatas).
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D4947?vs=125
jsalatas marked 9 inline comments as done.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: jsalatas, #frameworks, tfry, mwolff, #ktexteditor, cullmann
Cc: cullmann, dhaumann, anthonyfieroni, mwolff, kwrite-devel
jsalatas marked 2 inline comments as done.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: jsalatas, #frameworks, tfry, mwolff, #ktexteditor, cullmann
Cc: cullmann, dhaumann, anthonyfieroni, mwolff, kwrite-devel
cullmann accepted this revision.
cullmann added a comment.
Then I would say that is OK to go in.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: jsalatas, #frameworks, tfry, mwolff, #ktexteditor, cullmann
Cc: cullmann, dhaumann, anthonyfieroni, mwolff, kw
jsalatas added a comment.
In https://phabricator.kde.org/D4947#95893, @cullmann wrote:
> Sorry for late response, thanks for ping!
No worries :)
> First about the interface you added: I think that it is ok the way, beside
for the enum values I would go for CamelCase like all
jsalatas updated this revision to Diff 12598.
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D4947?vs=12416&id=12598
REVISION DETAIL
https://phabricator.kde.org/D4947
AFFECTED FILES
src/include/ktexteditor/view.h
src/utils/ktexteditor.cpp
src/view/k
cullmann added a comment.
Sorry for late response, thanks for ping!
First about the interface you added: I think that it is ok the way, beside
for the enum values I would go for CamelCase like all other things and
something like VisibleLine or RealLine, that is more KF/Qt style I would s
jsalatas added a comment.
ping?
:)
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: jsalatas, #frameworks, tfry, mwolff, #ktexteditor
Cc: cullmann, dhaumann, anthonyfieroni, mwolff, kwrite-devel
jsalatas added a comment.
In https://phabricator.kde.org/D4947#94562, @dhaumann wrote:
> Reading this API, I still have some general thoughts:
> This is the first time we expose the concept of "visible lines". So far,
this only exists internally in katetextfolding.h/cpp. By itself, thi
jsalatas updated this revision to Diff 12416.
jsalatas edited the summary of this revision.
jsalatas added a comment.
According to @cullmann's and @dhaumann's feedback created the `enum LineType`
and also renamed `firstVisibleLine()` and `lastVisibleLine()` to
`firstDisplayedLine()` and `last
dhaumann added a comment.
Reading this API, I still have some general thoughts:
This is the first time we expose the concept of "visible lines". So far, this
only exists internally in katetextfolding.h/cpp. By itself, this is fine, but
given we do not expose folding information so far, I th
cullmann added a comment.
Perhaps we should introduce some enum for real vs. visible lines.
The bool parameter looks not that understandable (yeah, we did that
internally already wrong).
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: jsalatas, #framew
jsalatas updated this revision to Diff 12412.
jsalatas added a comment.
in `firstVisibleLine()` and `lastVisibleLine()` provide the option for real
lines (in cases of folded text).
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D4947?vs=12396&id=12412
R
jsalatas updated this revision to Diff 12396.
jsalatas added a comment.
Implemented additional functions as non-virtuals, according to @cullmann's
suggestion.
It's much better and cleaner this way!
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D494
cullmann added a comment.
Hmm, for the interface thing: You could avoid that entirely if you just add
the functions as non-virtual ones.
That is now possible in KF5, e.g. see insertTemplate(...) method.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: j
jsalatas added a comment.
ping?
:)
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: jsalatas, #frameworks, tfry, mwolff, #ktexteditor
Cc: dhaumann, anthonyfieroni, mwolff, kwrite-devel
jsalatas updated this revision to Diff 12253.
jsalatas added a comment.
Diff updated. Hope I addressed all issues :\
See some additional comments/clarifications below:
In https://phabricator.kde.org/D4947#92889, @mwolff wrote:
> should be scrollToPos, c -> cursor, and make the cu
jsalatas added a comment.
Thank you all for your feedback. I'll update soon
INLINE COMMENTS
> mwolff wrote in view.h:479
> why is this not the document end pos? because of virtual cursors? if so,
> please rename this to `viewEndCursor()` or similar, I don't see why this
> should be call
tfry added a comment.
Hi thanks for adding me in CC. My use case was saving and restoring a certain
scroll position. Thus, what I am missing, immediately, is a way to retrieve the
current scroll position. The proposed startLine() goes some length, here, but
does not provide columns (in case
mwolff added a comment.
In https://phabricator.kde.org/D4947#92995, @anthonyfieroni wrote:
> Milian had in mind that you can take charge of Kompare and implement your
ideas rather than release new app. :)
Hihi, while not really my intention, that would of course the best way
forw
dhaumann added a reviewer: tfry.
dhaumann added a comment.
Add Thomas, since he iirc also was interested in such an api once.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: jsalatas, #ktexteditor, #frameworks, mwolff, tfry
Cc: dhaumann, anthonyfieroni, m
dhaumann added inline comments.
INLINE COMMENTS
> mwolff wrote in view.h:464
> should be scrollToPos, c -> cursor, and make the cursor const
I would even favour setScrollPosition(). Avoid abbreviations whenever possible.
And we already have many other API that contains the full word position.
jsalatas added a comment.
In https://phabricator.kde.org/D4947#92995, @anthonyfieroni wrote:
> Milian had in mind that you can take charge of Kompare and implement your
ideas rather than release new app. :)
lol! OK! Got it! I guess I can do that, although my initial motivation is
anthonyfieroni added a comment.
Milian had in mind that you can take charge of Kompare and implement your
ideas rather than release new app. :)
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4947
To: jsalatas, #ktexteditor, #frameworks, mwolff
Cc: anthonyfieroni
jsalatas added a comment.
In https://phabricator.kde.org/D4947#92889, @mwolff wrote:
> You will have to create a separate interface for this, similar to what we
have done in the past, with a TODO note that this should be merged in time for
KF6.
You mean an interface similar to Te
mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
now that this API becomes public, it must be improved to make it better
understandable to the public
and note that you cannot add new virtual methods to this interface, as it
jsalatas created this revision.
Restricted Application added a subscriber: kwrite-devel.
Restricted Application added a project: Frameworks.
REVISION SUMMARY
I'm creating a diff/patch frontend similar to Kompare but using KTextEditor
to display/edit source/destination files. In order to be able
27 matches
Mail list logo
