[education/rkward] rkward: Fix nextChunk()/prevChunk()
Thomas Friedrichsmeier
null at kde.org
Mon May 26 15:06:38 BST 2025
Git commit eb4a47587e85a36159ce14ebcb7929c310632791 by Thomas Friedrichsmeier.
Committed on 26/05/2025 at 14:06.
Pushed by tfry into branch 'master'.
Fix nextChunk()/prevChunk()
M +1 -1 rkward/autotests/data/script1.Rmd
M +24 -19 rkward/autotests/rkparsedscript_test.cpp
M +37 -23 rkward/misc/rkparsedscript.cpp
M +4 -0 rkward/windows/rkcommandeditorwindow.cpp
https://invent.kde.org/education/rkward/-/commit/eb4a47587e85a36159ce14ebcb7929c310632791
diff --git a/rkward/autotests/data/script1.Rmd b/rkward/autotests/data/script1.Rmd
index f85836835..70540faf2 100644
--- a/rkward/autotests/data/script1.Rmd
+++ b/rkward/autotests/data/script1.Rmd
@@ -9,7 +9,7 @@ This is markdown ``{r} .some <- `markdown` `` and this is too, with some `inline
Usually, there would be lot of mardown (and this may contain many relevant markers, such as {}, and []).
-```{r}
+```{r echo=TRUE}
symb01 <-symb02()
symb03 <- symb04
symb05(symb06); symb07
diff --git a/rkward/autotests/rkparsedscript_test.cpp b/rkward/autotests/rkparsedscript_test.cpp
index 6e07e89a6..47ccb57bf 100644
--- a/rkward/autotests/rkparsedscript_test.cpp
+++ b/rkward/autotests/rkparsedscript_test.cpp
@@ -34,6 +34,8 @@ void RKDebug(int, int level, const char *fmt, ...) {
va_end(ap);
}
+#define moveAndCheck(X, Y) moveAndCheckHelper(X, Y, __LINE__)
+
/** Test RKParsedScript class. */
class RKParsedScriptTest : public QObject {
Q_OBJECT
@@ -49,13 +51,17 @@ class RKParsedScriptTest : public QObject {
ps = RKParsedScript(script, rmd);
}
- void compareScript(RKParsedScript::ContextIndex pos, const QString &expected) {
+ void compareScript(RKParsedScript::ContextIndex pos, const QString &expected, int line) {
auto ctx = ps.getContext(pos);
- QCOMPARE(script.mid(ctx.start, expected.length()), expected);
+ const auto found = script.mid(ctx.start, expected.length());
+ if (found != expected) {
+ qDebug(" -- Real line number of following mismatch is %d -- ", line);
+ QCOMPARE(found, expected);
+ }
}
- RKParsedScript::ContextIndex moveAndCheck(const RKParsedScript::ContextIndex newpos, const QString &expected) {
- compareScript(newpos, expected);
+ RKParsedScript::ContextIndex moveAndCheckHelper(const RKParsedScript::ContextIndex newpos, const QString &expected, int line) {
+ compareScript(newpos, expected, line);
return newpos;
}
@@ -147,7 +153,6 @@ class RKParsedScriptTest : public QObject {
// ps.serialize();
auto ctx = ps.contextAtPos(0);
QVERIFY(ctx.valid());
- testLog("Block1");
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol00"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol01"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol19"_s);
@@ -155,14 +160,12 @@ class RKParsedScriptTest : public QObject {
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
- testLog("Block2");
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
- testLog("Block3");
ctx = ps.contextAtPos(script.indexOf(u"Symbol11"));
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol13"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol15"_s);
@@ -170,7 +173,6 @@ class RKParsedScriptTest : public QObject {
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol18"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol19"_s);
- testLog("Block4");
const auto symb18 = ps.contextAtPos(script.indexOf(u"Symbol18"));
ctx = moveAndCheck(ps.prevStatement(symb18), u"Symbol16"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol15"_s);
@@ -182,24 +184,20 @@ class RKParsedScriptTest : public QObject {
ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol03"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol01"_s);
- testLog("Block5");
const auto symb14 = ps.contextAtPos(script.indexOf(u"Symbol14"));
ctx = moveAndCheck(ps.prevStatement(symb14), u"Symbol11"_s); // shall stay in parenthesis
ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol10"_s); // shall move out
- testLog("Block6");
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
- testLog("Block7");
ctx = ps.contextAtPos(script.indexOf(u"{ aaa"));
ctx = moveAndCheck(ps.nextStatement(ctx), u"{"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"ddd"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"eee"_s);
ctx = moveAndCheck(ps.nextStatement(ctx), u"jjj"_s);
- testLog("Block7");
const auto symbjjj = ps.contextAtPos(script.indexOf(u"jjj"));
ctx = moveAndCheck(ps.prevStatement(symbjjj), u"eee"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"ddd"_s);
@@ -209,7 +207,6 @@ class RKParsedScriptTest : public QObject {
ctx = moveAndCheck(ps.prevStatement(ctx), u"Argname"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"FunctionList"_s);
- testLog("Block8");
const auto symbnest3 = ps.contextAtPos(script.indexOf(u"nest3"));
ctx = moveAndCheck(ps.prevStatement(symbnest3), u"nest2"_s);
ctx = moveAndCheck(ps.prevStatement(ctx), u"{"_s);
@@ -221,13 +218,12 @@ class RKParsedScriptTest : public QObject {
void nextPrevOuter() {
loadScript(u"script1.R"_s);
- testLog("Block1");
+
auto ctx = ps.contextAtPos(script.indexOf(u"nest3"));
ctx = moveAndCheck(ps.nextOuter(ctx), u"nest5"_s);
ctx = moveAndCheck(ps.nextOuter(ctx), u"ddd"_s);
QVERIFY(!ps.nextOuter(ctx).valid());
- testLog("Block2");
ctx = ps.contextAtPos(script.indexOf(u"nest3"));
ctx = moveAndCheck(ps.prevOuter(ctx), u"{"_s);
ctx = moveAndCheck(ps.prevOuter(ctx), u"{"_s);
@@ -238,14 +234,13 @@ class RKParsedScriptTest : public QObject {
void nextPrevToplevel() {
loadScript(u"script1.R"_s);
- testLog("Block1");
+
auto ctx = ps.contextAtPos(script.indexOf(u"Symbol09"));
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);
QVERIFY(!ps.nextToplevel(ctx).valid());
- testLog("Block2");
ctx = ps.contextAtPos(script.indexOf(u"Symbol09"));
ctx = moveAndCheck(ps.prevToplevel(ctx), u"Symbol01"_s);
ctx = moveAndCheck(ps.prevToplevel(ctx), u"Symbol00"_s);
@@ -256,14 +251,13 @@ class RKParsedScriptTest : public QObject {
void nextPrevInner() {
loadScript(u"script1.R"_s);
- testLog("Block1");
+
auto ctx = ps.contextAtPos(script.indexOf(u"nest5"));
ctx = moveAndCheck(ps.nextStatementOrInner(ctx), u"ddd"_s);
ctx = moveAndCheck(ps.nextStatementOrInner(ctx), u"eee"_s);
ctx = moveAndCheck(ps.nextStatementOrInner(ctx), u"ggg"_s);
ctx = moveAndCheck(ps.nextStatementOrInner(ctx), u"jjj"_s);
- testLog("Block2");
ctx = ps.contextAtPos(script.indexOf(u"jjj"));
ctx = moveAndCheck(ps.prevStatementOrInner(ctx), u"ggg"_s);
ctx = moveAndCheck(ps.prevStatementOrInner(ctx), u"eee"_s); // TODO: or should it be "fff"?
@@ -274,6 +268,7 @@ class RKParsedScriptTest : public QObject {
void range() {
loadScript(u"script1.R"_s);
+
auto ctx = ps.contextAtPos(script.indexOf(u"Symbol01"));
QCOMPARE(script.mid(ps.lastPositionInStatement(ctx) + 1, 9), u"\nSymbol19"_s);
ctx = ps.contextAtPos(script.indexOf(u"Symbol08"));
@@ -283,6 +278,7 @@ class RKParsedScriptTest : public QObject {
void rmdTest() {
loadScript(u"script1.Rmd"_s, true);
sanityTestHelper();
+
auto ctx = ps.contextAtPos(script.indexOf(u".some"));
QVERIFY(!ps.nextStatement(ctx).valid());
@@ -306,12 +302,21 @@ class RKParsedScriptTest : public QObject {
ctx = moveAndCheck(ps.prevStatement(ctx), u"symb11"_s);
QVERIFY(!ps.prevStatement(ctx).valid());
+ ctx = ps.contextAtPos(script.indexOf(u"This is markdown"));
+ ctx = moveAndCheck(ps.nextCodeChunk(ctx), u".some"_s);
+
+ ctx = ps.contextAtPos(script.indexOf(u"and this is too"));
+ ctx = moveAndCheck(ps.prevCodeChunk(ctx), u".some"_s);
+
ctx = ps.contextAtPos(script.indexOf(u".some"));
ctx = moveAndCheck(ps.nextCodeChunk(ctx), u"inline"_s);
ctx = moveAndCheck(ps.nextCodeChunk(ctx), u"symb01"_s);
ctx = moveAndCheck(ps.nextCodeChunk(ctx), u"symb11"_s);
QVERIFY(!ps.nextCodeChunk(ctx).valid());
+ ctx = ps.contextAtPos(script.indexOf(u" .some"));
+ ctx = moveAndCheck(ps.nextCodeChunk(ctx), u"inline"_s);
+
ctx = ps.contextAtPos(script.indexOf(u"symb13"));
ctx = moveAndCheck(ps.prevCodeChunk(ctx), u"symb01"_s);
ctx = moveAndCheck(ps.prevCodeChunk(ctx), u"inline"_s);
diff --git a/rkward/misc/rkparsedscript.cpp b/rkward/misc/rkparsedscript.cpp
index 956641b79..521a6f546 100644
--- a/rkward/misc/rkparsedscript.cpp
+++ b/rkward/misc/rkparsedscript.cpp
@@ -335,41 +335,55 @@ RKParsedScript::ContextIndex RKParsedScript::prevOuter(const ContextIndex from)
RKParsedScript::ContextIndex RKParsedScript::nextCodeChunk(const ContextIndex from) const {
RK_TRACE(MISC);
if (!from.valid()) return ContextIndex();
+
+ // The start of a code chunk is reliably defined by non-zero-length Top-Region
// NOTE: not using nextContext() for iterating, here, as that stops at Top regions.
unsigned int i = from.index;
- do {
- if (context_list.at(i).type == Top) break;
- } while (++i < context_list.size());
- // yes, we want this twice, as chunks are delimited by a top context at start and end
while (++i < context_list.size()) {
- if (context_list.at(i).type == Top) break;
+ const auto ctx = context_list.at(i);
+ if (ctx.type == Top && ctx.end > ctx.start) {
+ // Found it. Now skip past initial delimiters
+ do {
+ ++i;
+ } while (i < context_list.size() && context_list.at(i).type == Delimiter);
+ return ContextIndex(i < context_list.size() ? i : -1);
+ }
}
- do {
- ++i;
- } while (i < context_list.size() && context_list.at(i).type == Delimiter);
- if (i < context_list.size()) return ContextIndex(i);
return ContextIndex();
}
RKParsedScript::ContextIndex RKParsedScript::prevCodeChunk(const ContextIndex from) const {
RK_TRACE(MISC);
if (!from.valid()) return ContextIndex();
- // NOTE: not using nextContext() for iterating, here, as that stops at Top regions.
+
+ // For general logic, see nextCodeChunk(), above. We need to reverse across two chunk starts, here
+ // If we are *inside* a code chunk, we need to reverse to starts, else just one.
+ int chunkstarts = 1;
+ auto parent = from;
+ while (parent.valid()) {
+ parent = parentRegion(parent);
+ if (getContext(parent).type == Top) {
+ chunkstarts = 2;
+ break;
+ }
+ }
+
int i = from.index;
- do {
- if (context_list.at(i).type == Top) break;
- } while (--i >= 0);
- // yes, we want this twice, as chunks are delimited by a top context at start and end
- while (--i >= 0) {
- if (context_list.at(i).type == Top) break;
+ for (; chunkstarts > 0; --chunkstarts) {
+ while (--i >= 0) {
+ const auto ctx = context_list.at(i);
+ if (ctx.type == Top && ctx.end > ctx.start) {
+ break;
+ }
+ }
}
- do {
- --i;
- } while (i >= 0 && context_list.at(i).type != Top);
- do {
- ++i;
- } while (i < context_list.size() && context_list.at(i).type == Delimiter);
- return ContextIndex(i);
+ if (i >= 0) {
+ do {
+ ++i;
+ } while (i < context_list.size() && context_list.at(i).type == Delimiter);
+ return ContextIndex(i < context_list.size() ? i : -1);
+ }
+ return ContextIndex();
}
RKParsedScript::ContextIndex RKParsedScript::nextToplevel(const ContextIndex from) const {
diff --git a/rkward/windows/rkcommandeditorwindow.cpp b/rkward/windows/rkcommandeditorwindow.cpp
index 33f4472bb..b7f75f4f2 100644
--- a/rkward/windows/rkcommandeditorwindow.cpp
+++ b/rkward/windows/rkcommandeditorwindow.cpp
@@ -154,6 +154,10 @@ class RKCodeNavigation : public QWidget {
newpos.pos = ps.getContext(ps.nextToplevel(ci)).start;
} else if (command == u'T') {
newpos.pos = ps.getContext(ps.prevToplevel(ci)).start;
+ } else if (command == u'c') {
+ newpos.pos = ps.getContext(ps.nextCodeChunk(ci)).start;
+ } else if (command == u'C') {
+ newpos.pos = ps.getContext(ps.prevCodeChunk(ci)).start;
} else if (command == u's') {
auto posa = ps.getContext(ps.firstContextInStatement(ci)).start;
auto posb = ps.lastPositionInStatement(ci);
More information about the rkward-tracker
mailing list