[education/rkward] rkward: Proof of concept next-statement navigation (still a bit buggy)

Thomas Friedrichsmeier null at kde.org
Mon May 26 15:06:38 BST 2025


Git commit 24a3ce54c9375908a1b1c5c58ba1b1c0e7e5a3a0 by Thomas Friedrichsmeier.
Committed on 26/05/2025 at 14:06.
Pushed by tfry into branch 'master'.

Proof of concept next-statement navigation (still a bit buggy)

M  +77   -6    rkward/misc/rkparsedscript.cpp
M  +38   -6    rkward/misc/rkparsedscript.h
M  +21   -4    rkward/windows/rkcommandeditorwindow.cpp

https://invent.kde.org/education/rkward/-/commit/24a3ce54c9375908a1b1c5c58ba1b1c0e7e5a3a0

diff --git a/rkward/misc/rkparsedscript.cpp b/rkward/misc/rkparsedscript.cpp
index 96b1c89d4..11363e55f 100644
--- a/rkward/misc/rkparsedscript.cpp
+++ b/rkward/misc/rkparsedscript.cpp
@@ -15,6 +15,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
 RKParsedScript::RKParsedScript(const QString &content) {
 	context_list.reserve(200); // just a very wild guess
 	addContext(Top, -1, content);
+	RK_ASSERT(context_list.front().end == content.size() -1);
 };
 
 int RKParsedScript::addContext(ContextType type, int start, const QString &content) {
@@ -22,14 +23,22 @@ int RKParsedScript::addContext(ContextType type, int start, const QString &conte
 
 	int index = context_list.size();
 	// some contexts need (or benefit from) special handling depending on the preceding context
-	if (type == OtherOperator && prevtype == OtherOperator) {
-		// Merge any two subsequent operators into one token
+	// TODO: maybe it would be easier to do this in a separte post-processing step
+	if ((type == prevtype) && (type == OtherOperator || type == Delimiter || type == Comment)) {
+		// 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)) {
 		// 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;
+			}
+		}
 		context_list.emplace_back(type, start); // end will be filled in, later
 	}
 
@@ -82,14 +91,76 @@ int RKParsedScript::addContext(ContextType type, int start, const QString &conte
 	return pos;
 };
 
-int RKParsedScript::contextAtPos(int pos) const {
+RKParsedScript::ContextIndex RKParsedScript::contextAtPos(int pos) const {
 	// Context 0 is Top, not really of interest
-	for (int i = 1; i < context_list.size(); ++i) {
+	for (unsigned int i = 1; i < context_list.size(); ++i) {
 		if (context_list.at(i).start > pos) {
-			return i - 1;
+			return ContextIndex(i - 1);
+		}
+	}
+	return ContextIndex(0);
+}
+
+RKParsedScript::ContextIndex RKParsedScript::nextContext(ContextIndex from) const {
+	int i = from.index + 1;
+	if (i >= context_list.size()) i = -1;
+	return ContextIndex(i);
+}
+
+RKParsedScript::ContextIndex RKParsedScript::prevContext(ContextIndex from) const {
+	int i = from.index - 1;
+	if (i >= context_list.size()) i = -1;
+	return ContextIndex(i);
+}
+
+/** Find the innermost region (Context::maybeNesting()) containing this context */
+RKParsedScript::ContextIndex RKParsedScript::parentRegion(ContextIndex from) const {
+	if (from.valid())  {
+		int startpos = context_list.at(from.index).start;
+		for (int i = from.index-1; i >= 0; --i) {
+			if (context_list.at(i).maybeNesting() && context_list.at(i).end >= startpos) {
+				return ContextIndex(i);
+			}
 		}
 	}
-	return 0;
+	return ContextIndex();
+}
+
+/** Find next sibling context (no inner or outer contexts) */
+RKParsedScript::ContextIndex RKParsedScript::nextSiblingOrOuter(ContextIndex from) const {
+	if (from.valid())  {
+		int endpos = context_list.at(from.index).end;
+		unsigned int i = from.index;
+		do {
+			++i;
+		} while (i < context_list.size() && context_list.at(i).start <= endpos); // advance, skipping child regions
+		return ContextIndex(i < context_list.size() ? i : -1);
+	}
+	return ContextIndex();
+}
+
+RKParsedScript::ContextIndex RKParsedScript::nextSibling(ContextIndex from) const {
+	auto candidate = nextSiblingOrOuter(from);
+	if (parentRegion(candidate) == parentRegion(from)) return candidate;
+	return ContextIndex();
+}
+
+RKParsedScript::ContextIndex RKParsedScript::prevSiblingOrOuter(ContextIndex from) const {
+	if (from.valid())  {
+		int startpos = context_list.at(from.index).start;
+		int i = from.index;
+		do {
+			--i;
+		} while (i >= 0 && context_list.at(i).end >= startpos); // advance, skipping child regions
+		return ContextIndex(i);
+	}
+	return ContextIndex();
+}
+
+RKParsedScript::ContextIndex RKParsedScript::prevSibling(ContextIndex from) const {
+	auto candidate = prevSiblingOrOuter(from);
+	if (parentRegion(candidate) == parentRegion(from)) return candidate;
+	return ContextIndex();
 }
 
 // NOTE: used in debugging, only
diff --git a/rkward/misc/rkparsedscript.h b/rkward/misc/rkparsedscript.h
index f08b5aa36..602c0d592 100644
--- a/rkward/misc/rkparsedscript.h
+++ b/rkward/misc/rkparsedscript.h
@@ -22,7 +22,7 @@ by start position.
 Inside this flat list, a child context is defined by starting after (or at) the parent's start, and ending before (or at) the parent's end. Child
 contexts are always found after their parent in the list.
 
-Type of context. Parenthesis, Brace, and Bracket are the only ContextType s that we actually consider as nested.
+Type of context. Parenthesis, Brace, and Bracket, and the outermost context (Top) are the only ContextType s that we actually consider as nested.
 */
 class RKParsedScript {
   public:
@@ -45,6 +45,7 @@ class RKParsedScript {
 	struct Context {
 		Context(ContextType type, int start) : type(type), start(start) {};
 		Context(ContextType type, int start, int end) : type(type), start(start), end(end) {};
+		bool maybeNesting() const { return (type == Parenthesis || type == Brace || type == Bracket || type == Top); };
 		ContextType type;
 		int start;
 		int end;
@@ -52,12 +53,43 @@ class RKParsedScript {
 
 	RKParsedScript(const QString &content);
 
-	/** Find the (index of the) innermost context containing pos.
+	enum SearchFlags {
+		NoFlags,
+		SkipInnerContexts = 1,
+		SearchBackwards = 2
+	};
+
+	/** Helper struct to enforce strict type checking between character position, and index of context */
+	struct ContextIndex {
+		ContextIndex() : index(-1) {};
+		explicit ContextIndex(int index) : index(index) {};
+		bool valid() const { return index >= 0; };
+		bool operator==(const ContextIndex &other) { return index == other.index; };
+		bool operator!=(const ContextIndex &other) { return index != other.index; };
+		int index;
+	};
+
+	/** Find the innermost context containing pos.
 	 *  returns the previous context, if no context actually contains this position (e.g. on a space) */
-	int contextAtPos(int pos) const;
-	
-	const Context &getContext(int index) const {
-		return context_list.at(index);
+	ContextIndex contextAtPos(int pos) const;
+	ContextIndex nextContext(ContextIndex from) const;
+	ContextIndex prevContext(ContextIndex from) const;
+
+	/** Find the innermost region (Context::maybeNesting()) containing this context */
+	ContextIndex parentRegion(ContextIndex from) const;
+
+	/** Find next sibling context (no inner or outer contexts) */
+	ContextIndex nextSibling(ContextIndex from) const;
+	ContextIndex prevSibling(ContextIndex from) const;
+
+	ContextIndex nextSiblingOrOuter(ContextIndex from) const;
+	ContextIndex prevSiblingOrOuter(ContextIndex from) const;
+
+	/** retrieve the context at the given index. Safe to call, even with an invalid index
+	 *  (in which case the outermost context will be returned). */
+	const Context &getContext(ContextIndex index) const {
+		if (!index.valid()) return context_list.front();
+		return context_list.at(index.index);
 	}
 
   private:
diff --git a/rkward/windows/rkcommandeditorwindow.cpp b/rkward/windows/rkcommandeditorwindow.cpp
index 902b5666c..3bb997466 100644
--- a/rkward/windows/rkcommandeditorwindow.cpp
+++ b/rkward/windows/rkcommandeditorwindow.cpp
@@ -47,6 +47,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
 #include "../core/robjectlist.h"
 #include "../misc/rkcommonfunctions.h"
 #include "../misc/rkjobsequence.h"
+#include "../misc/rkparsedscript.h"
 #include "../misc/rkstandardactions.h"
 #include "../misc/rkstandardicons.h"
 #include "../misc/rkstyle.h"
@@ -102,7 +103,7 @@ class RKCodeNavigation : public QWidget {
 	void navigate(const QString &current) {
 		// TODO: cache the parse tree. But for testing, it's not so bad to have it all parsed per keypress
 		RKParsedScript tree(doc->text());
-//		qDebug("%s", qPrintable(tree.serialize()));
+		qDebug("%s", qPrintable(tree.serialize()));
 
 		// translate cursor position to string index
 		const auto cursor = view->cursorPosition();
@@ -113,15 +114,31 @@ class RKCodeNavigation : public QWidget {
 
 		// then find out, where that is in the parse tree
 		auto ci = tree.contextAtPos(pos);
+		auto cparent = tree.parentRegion(ci);
 
 		// apply navigation command
 		QChar command = current.back();
 		int newpos = pos;
 		if (command == u'n') {
-#warning Will overflow-crash and is not yet correct, either
-			 newpos = tree.getContext(ci+1).start;
+			auto ni = ci;
+
+			// skip over anyting that is not an explicit or implicit delimiter
+			while (true) {
+				ni = tree.nextSiblingOrOuter(ni);
+				if (!ni.valid()) break; // hit end
+				else if (tree.getContext(ni).type == RKParsedScript::Delimiter) break; 
+				else if (tree.getContext(ni).start > tree.getContext(cparent).end) break;
+			}
+
+			// skip over any following non-interesting contexts
+			while (true) {
+				auto type = tree.getContext(ni).type;
+				if (type != RKParsedScript::Delimiter && type != RKParsedScript::Comment) break;
+				ni = tree.nextSiblingOrOuter(ni);
+			}
+			newpos = tree.getContext(ni).start;
 		}
-		RK_DEBUG(COMMANDEDITOR, DL_WARNING, "navigate %d to %d", pos, newpos);
+		RK_DEBUG(COMMANDEDITOR, DL_DEBUG, "navigate %d to %d", pos, newpos);
 
 		// translate new position back to cursor coordinates
 		for (int l = 0; l < doc->lines(); ++l) {



More information about the rkward-tracker mailing list