[education/rkward] rkward: Fix navigation near top, bottom, and continuation over multiple newlines
Thomas Friedrichsmeier
null at kde.org
Mon May 26 15:06:38 BST 2025
Git commit ca1d169e613a1757701705997ed6caa81ea6488d by Thomas Friedrichsmeier.
Committed on 26/05/2025 at 14:06.
Pushed by tfry into branch 'master'.
Fix navigation near top, bottom, and continuation over multiple newlines
M +10 -1 rkward/autotests/data/script1.R
M +22 -5 rkward/autotests/rkparsedscript_test.cpp
M +23 -13 rkward/misc/rkparsedscript.cpp
M +1 -0 rkward/misc/rkparsedscript.h
https://invent.kde.org/education/rkward/-/commit/ca1d169e613a1757701705997ed6caa81ea6488d
diff --git a/rkward/autotests/data/script1.R b/rkward/autotests/data/script1.R
index f548c76af..78568c154 100644
--- a/rkward/autotests/data/script1.R
+++ b/rkward/autotests/data/script1.R
@@ -1,3 +1,4 @@
+symb.first
# - This file is part of the RKWard project (https://rkward.kde.org).
# SPDX-FileCopyrightText: by Thomas Friedrichsmeier <thomas.friedrichsmeier at kdemail.net>
# SPDX-FileContributor: The RKWard Team <rkward-devel at kde.org>
@@ -16,7 +17,13 @@ Symbol19
# Comment 4
Symbol.x <- Symbol.y + Symbol.z +
- Symbol.y
+ Symbol.y |>
+
+ cont_after_newline() |>
+ # a comment
+
+ # a comment
+ cont_after_comments()
FunctionList <- list(
Argname=function() { statement },
@@ -31,3 +38,5 @@ FunctionList <- list(
nest5
}, ddd, eee <- fff(ggg + hhh) + iii, jjj)
)
+
+symb.last
diff --git a/rkward/autotests/rkparsedscript_test.cpp b/rkward/autotests/rkparsedscript_test.cpp
index 876c61775..62f491e7b 100644
--- a/rkward/autotests/rkparsedscript_test.cpp
+++ b/rkward/autotests/rkparsedscript_test.cpp
@@ -54,8 +54,10 @@ class RKParsedScriptTest : public QObject {
void compareScript(RKParsedScript::ContextIndex pos, const QString &expected, int line) {
auto ctx = ps.getContext(pos);
const auto found = script.mid(ctx.start, expected.length());
- if (found != expected) {
- qDebug(" -- Real line number of following mismatch is %d -- ", line);
+ if (!pos.valid() || found != expected) {
+ testLog(" -- Real line number of following mismatch is %d -- ", line);
+ //testLog("%s", ps.serialize());
+ QVERIFY(pos.valid());
QCOMPARE(found, expected);
}
}
@@ -160,13 +162,16 @@ class RKParsedScriptTest : public QObject {
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol19"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol.x"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"FunctionList"_s);
- QVERIFY(!ps.nextStatement(ctx).valid()); // NOTE this need not be set in stone, could e.g. wrap around
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"symb.last"_s);
+ QVERIFY(!ps.nextStatement(ctx).valid());
+ ctx = ps.contextAtPos(script.indexOf(u"FunctionList"));
ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol.x"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol19"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol01"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol00"_s);
- // QVERIFY(!ps.prevStatement(ctx).valid()); // not sure that we want this
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"symb.first"_s);
+ QVERIFY(!ps.prevStatement(ctx).valid());
ctx = ps.contextAtPos(script.indexOf(u"Symbol11"));
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol13"_s);
@@ -192,7 +197,7 @@ class RKParsedScriptTest : public QObject {
ctx = ps.contextAtPos(script.indexOf(u"Argname"));
ctx = moveAndCheck(ps.nextStatement(ctx), u"makeFunction"_s);
- QVERIFY(!ps.nextStatement(ctx).valid()); // NOTE this need not be set in stone, could e.g. wrap around
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"symb.last"_s);
ctx = ps.contextAtPos(script.indexOf(u"{ aaa"));
ctx = moveAndCheck(ps.nextStatement(ctx), u"{"_s);
@@ -216,6 +221,15 @@ class RKParsedScriptTest : public QObject {
ctx = moveAndCheck(ps.prevStatement(ctx), u"{"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"{ aaa"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"makeFunction"_s);
+
+ // check navigation near the top and bottom, spefically
+ ctx = ps.contextAtPos(script.indexOf(u"symb.last"));
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"FunctionList"_s);
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"symb.last"_s);
+
+ ctx = ps.contextAtPos(script.indexOf(u"Symbol00"));
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"symb.first"_s);
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol00"_s);
}
void nextPrevOuter() {
@@ -224,6 +238,7 @@ class RKParsedScriptTest : public QObject {
auto ctx = ps.contextAtPos(script.indexOf(u"nest3"));
ctx = moveAndCheck(ps.nextOuter(ctx), u"nest5"_s);
ctx = moveAndCheck(ps.nextOuter(ctx), u"ddd"_s);
+ ctx = moveAndCheck(ps.nextOuter(ctx), u"symb.last"_s);
QVERIFY(!ps.nextOuter(ctx).valid());
ctx = ps.contextAtPos(script.indexOf(u"nest3"));
@@ -241,6 +256,7 @@ class RKParsedScriptTest : public QObject {
ctx = moveAndCheck(ps.nextToplevel(ctx), u"Symbol19"_s);
ctx = moveAndCheck(ps.nextToplevel(ctx), u"Symbol.x"_s);
ctx = moveAndCheck(ps.nextToplevel(ctx), u"FunctionList"_s);
+ ctx = moveAndCheck(ps.nextToplevel(ctx), u"symb.last"_s);
QVERIFY(!ps.nextToplevel(ctx).valid());
ctx = ps.contextAtPos(script.indexOf(u"Symbol09"));
@@ -248,6 +264,7 @@ class RKParsedScriptTest : public QObject {
ctx = moveAndCheck(ps.prevToplevel(ctx), u"Symbol00"_s);
ctx = moveAndCheck(ps.nextToplevel(ctx), u"Symbol01"_s); // make sure, we can still advance from here
ctx = moveAndCheck(ps.prevToplevel(ctx), u"Symbol00"_s);
+ ctx = moveAndCheck(ps.prevToplevel(ctx), u"symb.first"_s);
QVERIFY(!ps.prevToplevel(ctx).valid());
}
diff --git a/rkward/misc/rkparsedscript.cpp b/rkward/misc/rkparsedscript.cpp
index 83f3de328..497348e8b 100644
--- a/rkward/misc/rkparsedscript.cpp
+++ b/rkward/misc/rkparsedscript.cpp
@@ -12,7 +12,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
#include "../debug.h"
-RKParsedScript::RKParsedScript(const QString &content, bool rmd) : prevtype(None), allow_merge(true) {
+RKParsedScript::RKParsedScript(const QString &content, bool rmd) : prevtype(None), allow_merge(true), on_operator_rhs(false) {
RK_TRACE(MISC);
context_list.reserve(200); // just a very wild guess.
@@ -63,7 +63,6 @@ int RKParsedScript::addNextMarkdownChunk(int start, const QString &content) {
if (chunkend < 0) chunkend = content.size();
context_list.emplace_back(Comment, start, chunkstart - 1);
- prevtype = Comment; // causes insertion of a delimiter in the following addContext
addContext(Top, chunkstart - 1, content.left(chunkend)); // in case markdown region has incomplete syntax
// limit parsing to the actual markdown region
context_list.emplace_back(Top, chunkend, chunkend); // HACK: Used as a dummy separtor, here...
@@ -72,6 +71,11 @@ int RKParsedScript::addNextMarkdownChunk(int start, const QString &content) {
int RKParsedScript::addContext(ContextType type, int start, const QString &content) {
RK_TRACE(MISC);
+
+ if (prevtype == OtherOperator || prevtype == SubsetOperator) {
+ on_operator_rhs = true;
+ }
+
int index = context_list.size();
// some contexts need (or benefit from) special handling depending on the preceding context
// TODO: maybe it would be easier to do this in a separte post-processing step
@@ -79,21 +83,27 @@ int RKParsedScript::addContext(ContextType type, int start, const QString &conte
// Merge any two subsequent operators or delimiters, and contiguous comment regions into one token
// i.e. do not add a context, we'll reuse the previous one.
--index;
- } else if (type == Delimiter && content.at(start) == u'\n' && (prevtype == OtherOperator || prevtype == SubsetOperator)) {
+ } else if (type == Delimiter && content.at(start) == u'\n' && on_operator_rhs) {
// newlines do not count as delimiter on operator RHS, so skip ahead, instead of really adding this
return start;
} else {
- if (prevtype == Comment) { // comment region implies a newline
- auto pprevtype = context_list.at(index - 2).type;
- if (pprevtype != OtherOperator && pprevtype != SubsetOperator) {
- context_list.emplace_back(Delimiter, start - 1, start - 1);
- ++index;
- }
+ // comment region implies a newline, i.e. a delimiter in most scenarios
+ if (prevtype == Comment && !on_operator_rhs) {
+ context_list.emplace_back(Delimiter, start - 1, start - 1);
+ ++index;
}
context_list.emplace_back(type, start); // end will be filled in, later
+ if (type == Top) {
+ // we also insert a delimiter right after Top to help our algorithms
+ context_list.emplace_back(Delimiter, start - 1, start - 1);
+ }
}
allow_merge = true;
+ if (!(type == OtherOperator || type == SubsetOperator || type == Delimiter || type == Comment)) {
+ on_operator_rhs = false;
+ }
+
int pos = start;
if (type == SingleQuoted || type == DoubleQuoted || type == BackQuoted) {
while (++pos < content.length()) {
@@ -380,8 +390,8 @@ RKParsedScript::ContextIndex RKParsedScript::prevCodeChunk(const ContextIndex fr
if (i >= 0) {
do {
++i;
- } while (i < context_list.size() && context_list.at(i).type == Delimiter);
- return ContextIndex(i < context_list.size() ? i : -1);
+ } while (i < (int) context_list.size() && context_list.at(i).type == Delimiter);
+ return ContextIndex(i < (int) context_list.size() ? i : -1);
}
return ContextIndex();
}
@@ -399,8 +409,8 @@ RKParsedScript::ContextIndex RKParsedScript::firstContextInChunk(const ContextIn
int i = parent.index;
do {
++i;
- } while (i < context_list.size() && context_list.at(i).type == Delimiter);
- if (i >= context_list.size()) return ContextIndex();
+ } while (i < (int) context_list.size() && context_list.at(i).type == Delimiter);
+ if (i >= (int) context_list.size()) return ContextIndex();
return ContextIndex(i);
}
diff --git a/rkward/misc/rkparsedscript.h b/rkward/misc/rkparsedscript.h
index 800f05bc2..197d5bd07 100644
--- a/rkward/misc/rkparsedscript.h
+++ b/rkward/misc/rkparsedscript.h
@@ -135,6 +135,7 @@ class RKParsedScript {
std::vector<Context> context_list;
ContextType prevtype;
bool allow_merge;
+ bool on_operator_rhs;
};
#endif
More information about the rkward-tracker
mailing list