[education/rkward] rkward: Fix regular forward navigation, and add backward navigation

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


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

Fix regular forward navigation, and add backward navigation

M  +76   -13   rkward/misc/rkparsedscript.cpp
M  +13   -8    rkward/misc/rkparsedscript.h
M  +3    -18   rkward/windows/rkcommandeditorwindow.cpp

https://invent.kde.org/education/rkward/-/commit/0988e9d42662ea69aefe146af96cec5e8d04cdf2

diff --git a/rkward/misc/rkparsedscript.cpp b/rkward/misc/rkparsedscript.cpp
index 11363e55f..307db4382 100644
--- a/rkward/misc/rkparsedscript.cpp
+++ b/rkward/misc/rkparsedscript.cpp
@@ -12,19 +12,17 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "../debug.h"
 
-RKParsedScript::RKParsedScript(const QString &content) {
+RKParsedScript::RKParsedScript(const QString &content) : prevtype(None), allow_merge(true) {
 	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) {
-	ContextType prevtype = context_list.empty() ? None : context_list.back().type;
-
 	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
-	if ((type == prevtype) && (type == OtherOperator || type == Delimiter || type == Comment)) {
+	if (allow_merge && (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;
@@ -41,6 +39,7 @@ int RKParsedScript::addContext(ContextType type, int start, const QString &conte
 		}
 		context_list.emplace_back(type, start); // end will be filled in, later
 	}
+	allow_merge = true;
 
 	int pos = start;
 	if (type == SingleQuoted || type == DoubleQuoted || type == BackQuoted) {
@@ -85,9 +84,15 @@ int RKParsedScript::addContext(ContextType type, int start, const QString &conte
 		}
 	}
 
+	// do not merge comment regions etc across Brace ends
+	if (type == Parenthesis || type == Brace || type == Bracket) {
+		allow_merge = false;
+	}
+
 	// NOTE: we can't just keep a reference to the context at the start of this function, as the vector
 	//       may re-allocate during nested parsing
 	context_list.at(index).end = pos;
+	prevtype = type;
 	return pos;
 };
 
@@ -101,20 +106,20 @@ RKParsedScript::ContextIndex RKParsedScript::contextAtPos(int pos) const {
 	return ContextIndex(0);
 }
 
-RKParsedScript::ContextIndex RKParsedScript::nextContext(ContextIndex from) const {
+RKParsedScript::ContextIndex RKParsedScript::nextContext(const ContextIndex from) const {
 	int i = from.index + 1;
-	if (i >= context_list.size()) i = -1;
+	if (i >= (int) context_list.size()) i = -1;
 	return ContextIndex(i);
 }
 
-RKParsedScript::ContextIndex RKParsedScript::prevContext(ContextIndex from) const {
+RKParsedScript::ContextIndex RKParsedScript::prevContext(const ContextIndex from) const {
 	int i = from.index - 1;
-	if (i >= context_list.size()) i = -1;
+	if (i >= (int) context_list.size()) i = -1;
 	return ContextIndex(i);
 }
 
 /** Find the innermost region (Context::maybeNesting()) containing this context */
-RKParsedScript::ContextIndex RKParsedScript::parentRegion(ContextIndex from) const {
+RKParsedScript::ContextIndex RKParsedScript::parentRegion(const ContextIndex from) const {
 	if (from.valid())  {
 		int startpos = context_list.at(from.index).start;
 		for (int i = from.index-1; i >= 0; --i) {
@@ -127,7 +132,7 @@ RKParsedScript::ContextIndex RKParsedScript::parentRegion(ContextIndex from) con
 }
 
 /** Find next sibling context (no inner or outer contexts) */
-RKParsedScript::ContextIndex RKParsedScript::nextSiblingOrOuter(ContextIndex from) const {
+RKParsedScript::ContextIndex RKParsedScript::nextSiblingOrOuter(const ContextIndex from) const {
 	if (from.valid())  {
 		int endpos = context_list.at(from.index).end;
 		unsigned int i = from.index;
@@ -139,30 +144,87 @@ RKParsedScript::ContextIndex RKParsedScript::nextSiblingOrOuter(ContextIndex fro
 	return ContextIndex();
 }
 
-RKParsedScript::ContextIndex RKParsedScript::nextSibling(ContextIndex from) const {
+RKParsedScript::ContextIndex RKParsedScript::nextSibling(const ContextIndex from) const {
 	auto candidate = nextSiblingOrOuter(from);
 	if (parentRegion(candidate) == parentRegion(from)) return candidate;
 	return ContextIndex();
 }
 
-RKParsedScript::ContextIndex RKParsedScript::prevSiblingOrOuter(ContextIndex from) const {
+RKParsedScript::ContextIndex RKParsedScript::prevSiblingOrOuter(const 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
+
+		while (true) {
+			// handle the case of "a(b)c": Moving rev from c would give us b, however, that's a nephew, not a sibling
+			// In this situation we want to return b's parent (i.e. the opening brace)
+			// In a loop, as the situation could also be "a{(b)}c", etc.
+			auto candidate_parent = parentRegion(ContextIndex(i));
+			if (getContext(candidate_parent).end >= startpos) break;
+			i = candidate_parent.index;
+		}
 		return ContextIndex(i);
 	}
 	return ContextIndex();
 }
 
-RKParsedScript::ContextIndex RKParsedScript::prevSibling(ContextIndex from) const {
+RKParsedScript::ContextIndex RKParsedScript::prevSibling(const ContextIndex from) const {
 	auto candidate = prevSiblingOrOuter(from);
 	if (parentRegion(candidate) == parentRegion(from)) return candidate;
 	return ContextIndex();
 }
 
+RKParsedScript::ContextIndex RKParsedScript::nextStatement(const ContextIndex from) const {
+	auto ni = from;
+	auto cparent = parentRegion(from);
+
+	// skip over anything that is not an explicit or implicit delimiter
+	while (true) {
+		ni = nextSiblingOrOuter(ni);
+		if (!ni.valid()) break; // hit end
+		else if (getContext(ni).type == Delimiter) break;
+		else if (getContext(ni).start > getContext(cparent).end) break;
+	}
+
+	// skip over any following non-interesting contexts
+	while (true) {
+		auto type = getContext(ni).type;
+		if (type != Delimiter && type != Comment) break;
+		ni = nextSiblingOrOuter(ni);
+	}
+	return ni;
+}
+
+RKParsedScript::ContextIndex RKParsedScript::prevStatement(const ContextIndex from) const {
+	auto pi = from;
+
+	// TODO: refactor into more readable / reusable parts: statementStart() and statementEnd()
+
+	// find start of current statement (the delimiter, preceding it)
+	while (true) {
+		pi = prevSiblingOrOuter(pi);
+		if (!pi.valid()) break; // hit top end
+		else if (getContext(pi).type == Delimiter) break;
+	}
+	// from here, skip over any preceding non-interesting contexts (entering the previous statement
+	while (true) {
+		auto type = getContext(pi).type;
+		if (type != Delimiter && type != Comment) break;
+		pi = prevSiblingOrOuter(pi);
+	}
+	// now find the start of the previous statement
+	while (true) {
+		pi = prevSiblingOrOuter(pi);
+		if (!pi.valid()) break; // hit top end
+		else if (getContext(pi).type == Delimiter) break;
+	}
+	// actual start is the token after the delimiter
+	return nextContext(pi);
+}
+
 // NOTE: used in debugging, only
 QString RKParsedScript::serialize() const {
 	QString ret;
@@ -171,6 +233,7 @@ QString RKParsedScript::serialize() const {
 
 	for (unsigned int i = 0; i < context_list.size(); ++i) {
 		const auto ctx = context_list.at(i);
+		qDebug("ctx type %d: %d->%d\n", ctx.type, ctx.start, ctx.end);
 
 		// end any finished contexts
 		while (ctx.start >= stack.back().end) {
diff --git a/rkward/misc/rkparsedscript.h b/rkward/misc/rkparsedscript.h
index 602c0d592..95b156573 100644
--- a/rkward/misc/rkparsedscript.h
+++ b/rkward/misc/rkparsedscript.h
@@ -72,22 +72,25 @@ class RKParsedScript {
 	/** Find the innermost context containing pos.
 	 *  returns the previous context, if no context actually contains this position (e.g. on a space) */
 	ContextIndex contextAtPos(int pos) const;
-	ContextIndex nextContext(ContextIndex from) const;
-	ContextIndex prevContext(ContextIndex from) const;
+	ContextIndex nextContext(const ContextIndex from) const;
+	ContextIndex prevContext(const ContextIndex from) const;
 
 	/** Find the innermost region (Context::maybeNesting()) containing this context */
-	ContextIndex parentRegion(ContextIndex from) const;
+	ContextIndex parentRegion(const ContextIndex from) const;
 
 	/** Find next sibling context (no inner or outer contexts) */
-	ContextIndex nextSibling(ContextIndex from) const;
-	ContextIndex prevSibling(ContextIndex from) const;
+	ContextIndex nextSibling(const ContextIndex from) const;
+	ContextIndex prevSibling(const ContextIndex from) const;
 
-	ContextIndex nextSiblingOrOuter(ContextIndex from) const;
-	ContextIndex prevSiblingOrOuter(ContextIndex from) const;
+	ContextIndex nextSiblingOrOuter(const ContextIndex from) const;
+	ContextIndex prevSiblingOrOuter(const ContextIndex from) const;
+
+	ContextIndex nextStatement(const ContextIndex from) const;
+	ContextIndex prevStatement(const 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 {
+	const Context &getContext(const ContextIndex index) const {
 		if (!index.valid()) return context_list.front();
 		return context_list.at(index.index);
 	}
@@ -104,6 +107,8 @@ friend class RKCodeNavigation;
 	// I want to modify some objects in place during parsing, without triggering copy-on-write
 	// hence no Qt container
 	std::vector<Context> context_list;
+	ContextType prevtype;
+	bool allow_merge;
 };
 
 #endif
diff --git a/rkward/windows/rkcommandeditorwindow.cpp b/rkward/windows/rkcommandeditorwindow.cpp
index 3bb997466..531f9d5fa 100644
--- a/rkward/windows/rkcommandeditorwindow.cpp
+++ b/rkward/windows/rkcommandeditorwindow.cpp
@@ -114,29 +114,14 @@ 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') {
-			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;
+			newpos = tree.getContext(tree.nextStatement(ci)).start;
+		} else if (command == u'N') {
+			newpos = tree.getContext(tree.prevStatement(ci)).start;
 		}
 		RK_DEBUG(COMMANDEDITOR, DL_DEBUG, "navigate %d to %d", pos, newpos);
 



More information about the rkward-tracker mailing list