From 4b8568006d1f83d518d98ef3c7354b7c25de46cc Mon Sep 17 00:00:00 2001
From: Ignacio Etcheverry <ignalfonsore@gmail.com>
Date: Sat, 15 Apr 2017 19:48:10 +0200
Subject: [PATCH] External editor improvements and fixes

Notable changes:

- Now ScriptLanguages have the option to override the global external editor setting.
If `ScriptLanguage::open_in_external_editor()` returns `ERR_UNAVAILABLE` (which it does by default), then the global external editor option will be used.
- Added formatting to the external editor execution arguments. Now it's possible to write something like this: `{project} -g {file}:{line}:{col}`.
- `VisualScript::get_member_line()` now can return the line of functions (well, it returns the id of the _Function_ node of the function). I guess there is nothing else we can get a "line" from.

Fixes:

- Fixes a bug where `ScriptEditor::script_goto_method()` would not work if the script is not already open in the built-in editor.
- Fixes wrong DEFVAL for `cursor_set_column` and `cursor_set_line` in TextEdit.
- `Script::get_member_line()` now returns -1 ("found nothing") by default.
---
 core/script_language.h                        |  3 +-
 editor/connections_dialog.cpp                 |  2 +-
 editor/plugins/script_editor_plugin.cpp       | 72 +++++++++++++------
 editor/plugins/script_editor_plugin.h         |  7 +-
 editor/plugins/script_text_editor.cpp         | 22 +-----
 editor/plugins/script_text_editor.h           |  1 -
 modules/visual_script/visual_script.cpp       | 12 ++++
 modules/visual_script/visual_script.h         |  2 +
 .../visual_script/visual_script_editor.cpp    | 14 +---
 modules/visual_script/visual_script_editor.h  |  1 -
 scene/gui/text_edit.cpp                       |  4 +-
 11 files changed, 74 insertions(+), 66 deletions(-)

diff --git a/core/script_language.h b/core/script_language.h
index 905ac4eedaf..1ec02f5845c 100644
--- a/core/script_language.h
+++ b/core/script_language.h
@@ -119,7 +119,7 @@ public:
 	virtual void get_script_method_list(List<MethodInfo> *p_list) const = 0;
 	virtual void get_script_property_list(List<PropertyInfo> *p_list) const = 0;
 
-	virtual int get_member_line(const StringName &p_member) const { return 0; }
+	virtual int get_member_line(const StringName &p_member) const { return -1; }
 
 	Script() {}
 };
@@ -201,6 +201,7 @@ public:
 	virtual bool has_named_classes() const = 0;
 	virtual int find_function(const String &p_function, const String &p_code) const = 0;
 	virtual String make_function(const String &p_class, const String &p_name, const PoolStringArray &p_args) const = 0;
+	virtual Error open_in_external_editor(const Ref<Script> &p_script, int p_line, int p_col) { return ERR_UNAVAILABLE; }
 
 	virtual Error complete_code(const String &p_code, const String &p_base_path, Object *p_owner, List<String> *r_options, String &r_call_hint) { return ERR_UNAVAILABLE; }
 
diff --git a/editor/connections_dialog.cpp b/editor/connections_dialog.cpp
index 9c6624d9e55..93c2b7493c7 100644
--- a/editor/connections_dialog.cpp
+++ b/editor/connections_dialog.cpp
@@ -767,7 +767,7 @@ void ConnectionsDock::_something_activated() {
 
 		Ref<Script> script = c.target->get_script();
 
-		if (script.is_valid() && ScriptEditor::get_singleton()->script_go_to_method(script, c.method)) {
+		if (script.is_valid() && ScriptEditor::get_singleton()->script_goto_method(script, c.method)) {
 			editor->call("_editor_select", EditorNode::EDITOR_SCRIPT);
 		}
 	}
diff --git a/editor/plugins/script_editor_plugin.cpp b/editor/plugins/script_editor_plugin.cpp
index 8ce0f51211d..20ded1d8fd2 100644
--- a/editor/plugins/script_editor_plugin.cpp
+++ b/editor/plugins/script_editor_plugin.cpp
@@ -1380,10 +1380,10 @@ void ScriptEditor::_update_script_names() {
 	_update_script_colors();
 }
 
-void ScriptEditor::edit(const Ref<Script> &p_script, bool p_grab_focus) {
+bool ScriptEditor::edit(const Ref<Script> &p_script, int p_line, int p_col, bool p_grab_focus) {
 
 	if (p_script.is_null())
-		return;
+		return false;
 
 	// refuse to open built-in if scene is not loaded
 
@@ -1391,22 +1391,46 @@ void ScriptEditor::edit(const Ref<Script> &p_script, bool p_grab_focus) {
 
 	bool open_dominant = EditorSettings::get_singleton()->get("text_editor/files/open_dominant_script_on_scene_change");
 
+	Error err = p_script->get_language()->open_in_external_editor(p_script, p_line >= 0 ? p_line : 0, p_col);
+	if (err == OK)
+		return false;
+	if (err != ERR_UNAVAILABLE)
+		WARN_PRINT("Couldn't open in custom external text editor");
+
 	if (p_script->get_path().is_resource_file() && bool(EditorSettings::get_singleton()->get("text_editor/external/use_external_editor"))) {
 
 		String path = EditorSettings::get_singleton()->get("text_editor/external/exec_path");
 		String flags = EditorSettings::get_singleton()->get("text_editor/external/exec_flags");
+
+		Dictionary keys;
+		keys["project"] = GlobalConfig::get_singleton()->get_resource_path();
+		keys["file"] = GlobalConfig::get_singleton()->globalize_path(p_script->get_path());
+		keys["line"] = p_line >= 0 ? p_line : 0;
+		keys["col"] = p_col;
+
+		flags = flags.format(keys).strip_edges().replace("\\\\", "\\");
+
 		List<String> args;
-		flags = flags.strip_edges();
-		if (flags != String()) {
-			Vector<String> flagss = flags.split(" ", false);
-			for (int i = 0; i < flagss.size(); i++)
-				args.push_back(flagss[i]);
+
+		if (flags.size()) {
+			int from = 0, to = 0;
+			bool inside_quotes = false;
+			for (int i = 0; i < flags.size(); i++) {
+				if (flags[i] == '"' && (!i || flags[i - 1] != '\\')) {
+					inside_quotes = !inside_quotes;
+				} else if (flags[i] == '\0' || (!inside_quotes && flags[i] == ' ')) {
+					args.push_back(flags.substr(from, to));
+					from = i + 1;
+					to = 0;
+				} else {
+					to++;
+				}
+			}
 		}
 
-		args.push_back(GlobalConfig::get_singleton()->globalize_path(p_script->get_path()));
 		Error err = OS::get_singleton()->execute(path, args, false);
 		if (err == OK)
-			return;
+			return false;
 		WARN_PRINT("Couldn't open external text editor, using internal");
 	}
 
@@ -1425,8 +1449,11 @@ void ScriptEditor::edit(const Ref<Script> &p_script, bool p_grab_focus) {
 				}
 				if (is_visible_in_tree())
 					se->ensure_focus();
+
+				if (p_line >= 0)
+					se->goto_line(p_line - 1);
 			}
-			return;
+			return true;
 		}
 	}
 
@@ -1439,7 +1466,7 @@ void ScriptEditor::edit(const Ref<Script> &p_script, bool p_grab_focus) {
 		if (se)
 			break;
 	}
-	ERR_FAIL_COND(!se);
+	ERR_FAIL_COND_V(!se, false);
 	tab_container->add_child(se);
 
 	se->set_edited_script(p_script);
@@ -1466,6 +1493,11 @@ void ScriptEditor::edit(const Ref<Script> &p_script, bool p_grab_focus) {
 
 	_test_script_times_on_disk(p_script);
 	_update_modified_scripts_for_external_editor(p_script);
+
+	if (p_line >= 0)
+		se->goto_line(p_line - 1);
+
+	return true;
 }
 
 void ScriptEditor::save_all_scripts() {
@@ -1863,20 +1895,14 @@ void ScriptEditor::set_scene_root_script(Ref<Script> p_script) {
 	}
 }
 
-bool ScriptEditor::script_go_to_method(Ref<Script> p_script, const String &p_method) {
+bool ScriptEditor::script_goto_method(Ref<Script> p_script, const String &p_method) {
 
-	for (int i = 0; i < tab_container->get_child_count(); i++) {
-		ScriptEditorBase *current = tab_container->get_child(i)->cast_to<ScriptEditorBase>();
+	int line = p_script->get_member_line(p_method);
 
-		if (current && current->get_edited_script() == p_script) {
-			if (current->goto_method(p_method)) {
-				edit(p_script);
-				return true;
-			}
-			break;
-		}
-	}
-	return false;
+	if (line == -1)
+		return false;
+
+	return edit(p_script, line, 0);
 }
 
 void ScriptEditor::set_live_auto_reload_running_scripts(bool p_enabled) {
diff --git a/editor/plugins/script_editor_plugin.h b/editor/plugins/script_editor_plugin.h
index 51d9bd3fc8c..f3bb5b55461 100644
--- a/editor/plugins/script_editor_plugin.h
+++ b/editor/plugins/script_editor_plugin.h
@@ -95,7 +95,6 @@ public:
 	virtual void tag_saved_version() = 0;
 	virtual void reload(bool p_soft) = 0;
 	virtual void get_breakpoints(List<int> *p_breakpoints) = 0;
-	virtual bool goto_method(const String &p_method) = 0;
 	virtual void add_callback(const String &p_function, PoolStringArray p_args) = 0;
 	virtual void update_settings() = 0;
 	virtual void set_debugger_active(bool p_active) = 0;
@@ -312,7 +311,9 @@ public:
 	void apply_scripts() const;
 
 	void ensure_select_current();
-	void edit(const Ref<Script> &p_script, bool p_grab_focus = true);
+
+	_FORCE_INLINE_ bool edit(const Ref<Script> &p_script, bool p_grab_focus = true) { return edit(p_script, -1, 0, p_grab_focus); }
+	bool edit(const Ref<Script> &p_script, int p_line, int p_col, bool p_grab_focus = true);
 
 	Dictionary get_state() const;
 	void set_state(const Dictionary &p_state);
@@ -329,7 +330,7 @@ public:
 
 	void set_scene_root_script(Ref<Script> p_script);
 
-	bool script_go_to_method(Ref<Script> p_script, const String &p_method);
+	bool script_goto_method(Ref<Script> p_script, const String &p_method);
 
 	virtual void edited_scene_changed();
 
diff --git a/editor/plugins/script_text_editor.cpp b/editor/plugins/script_text_editor.cpp
index 159fc0d51f8..8119acac537 100644
--- a/editor/plugins/script_text_editor.cpp
+++ b/editor/plugins/script_text_editor.cpp
@@ -69,26 +69,6 @@ Ref<Script> ScriptTextEditor::get_edited_script() const {
 	return script;
 }
 
-bool ScriptTextEditor::goto_method(const String &p_method) {
-
-	Vector<String> functions = get_functions();
-
-	String method_search = p_method + ":";
-
-	for (int i = 0; i < functions.size(); i++) {
-		String function = functions[i];
-
-		if (function.begins_with(method_search)) {
-
-			int line = function.get_slice(":", 1).to_int();
-			goto_line(line - 1);
-			return true;
-		}
-	}
-
-	return false;
-}
-
 void ScriptTextEditor::_load_theme_settings() {
 
 	TextEdit *text_edit = code_editor->get_text_edit();
@@ -296,7 +276,7 @@ void ScriptTextEditor::tag_saved_version() {
 }
 
 void ScriptTextEditor::goto_line(int p_line, bool p_with_error) {
-	code_editor->get_text_edit()->cursor_set_line(p_line);
+	code_editor->get_text_edit()->call_deferred("cursor_set_line", p_line);
 }
 
 void ScriptTextEditor::ensure_focus() {
diff --git a/editor/plugins/script_text_editor.h b/editor/plugins/script_text_editor.h
index 0649e39ab74..4bf525e2d12 100644
--- a/editor/plugins/script_text_editor.h
+++ b/editor/plugins/script_text_editor.h
@@ -134,7 +134,6 @@ public:
 
 	virtual void add_callback(const String &p_function, PoolStringArray p_args);
 	virtual void update_settings();
-	virtual bool goto_method(const String &p_method);
 
 	virtual void set_tooltip_request_func(String p_method, Object *p_obj);
 
diff --git a/modules/visual_script/visual_script.cpp b/modules/visual_script/visual_script.cpp
index 53521c55a7d..aec60391d3e 100644
--- a/modules/visual_script/visual_script.cpp
+++ b/modules/visual_script/visual_script.cpp
@@ -1073,6 +1073,18 @@ void VisualScript::get_script_property_list(List<PropertyInfo> *p_list) const {
 	}
 }
 
+int VisualScript::get_member_line(const StringName &p_member) const {
+#ifdef TOOLS_ENABLED
+	if (has_function(p_member)) {
+		for (Map<int, Function::NodeData>::Element *E = functions[p_member].nodes.front(); E; E = E->next()) {
+			if (E->get().node->cast_to<VisualScriptFunction>())
+				return E->key();
+		}
+	}
+#endif
+	return -1;
+}
+
 #ifdef TOOLS_ENABLED
 bool VisualScript::are_subnodes_edited() const {
 
diff --git a/modules/visual_script/visual_script.h b/modules/visual_script/visual_script.h
index ee8ed2c965f..273a8196117 100644
--- a/modules/visual_script/visual_script.h
+++ b/modules/visual_script/visual_script.h
@@ -356,6 +356,8 @@ public:
 
 	virtual void get_script_property_list(List<PropertyInfo> *p_list) const;
 
+	virtual int get_member_line(const StringName &p_member) const;
+
 #ifdef TOOLS_ENABLED
 	virtual bool are_subnodes_edited() const;
 #endif
diff --git a/modules/visual_script/visual_script_editor.cpp b/modules/visual_script/visual_script_editor.cpp
index ee5bd66aa21..5a765f93424 100644
--- a/modules/visual_script/visual_script_editor.cpp
+++ b/modules/visual_script/visual_script_editor.cpp
@@ -2153,7 +2153,7 @@ void VisualScriptEditor::goto_line(int p_line, bool p_with_error) {
 			_update_graph();
 			_update_members();
 
-			call_deferred("_center_on_node", p_line); //editor might be just created and size might not exist yet
+			call_deferred("call_deferred", "_center_on_node", p_line); //editor might be just created and size might not exist yet
 
 			return;
 		}
@@ -2192,18 +2192,6 @@ void VisualScriptEditor::get_breakpoints(List<int> *p_breakpoints) {
 	}
 }
 
-bool VisualScriptEditor::goto_method(const String &p_method) {
-
-	if (!script->has_function(p_method))
-		return false;
-
-	edited_func = p_method;
-	selected = edited_func;
-	_update_members();
-	_update_graph();
-	return true;
-}
-
 void VisualScriptEditor::add_callback(const String &p_function, PoolStringArray p_args) {
 
 	if (script->has_function(p_function)) {
diff --git a/modules/visual_script/visual_script_editor.h b/modules/visual_script/visual_script_editor.h
index 7322da5cfb7..52f5d28a36c 100644
--- a/modules/visual_script/visual_script_editor.h
+++ b/modules/visual_script/visual_script_editor.h
@@ -244,7 +244,6 @@ public:
 	virtual void tag_saved_version();
 	virtual void reload(bool p_soft);
 	virtual void get_breakpoints(List<int> *p_breakpoints);
-	virtual bool goto_method(const String &p_method);
 	virtual void add_callback(const String &p_function, PoolStringArray p_args);
 	virtual void update_settings();
 	virtual void set_debugger_active(bool p_active);
diff --git a/scene/gui/text_edit.cpp b/scene/gui/text_edit.cpp
index 80c6a23b61c..8cbd6675f07 100644
--- a/scene/gui/text_edit.cpp
+++ b/scene/gui/text_edit.cpp
@@ -4448,8 +4448,8 @@ void TextEdit::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_text"), &TextEdit::get_text);
 	ClassDB::bind_method(D_METHOD("get_line", "line"), &TextEdit::get_line);
 
-	ClassDB::bind_method(D_METHOD("cursor_set_column", "column", "adjust_viewport"), &TextEdit::cursor_set_column, DEFVAL(false));
-	ClassDB::bind_method(D_METHOD("cursor_set_line", "line", "adjust_viewport"), &TextEdit::cursor_set_line, DEFVAL(false));
+	ClassDB::bind_method(D_METHOD("cursor_set_column", "column", "adjust_viewport"), &TextEdit::cursor_set_column, DEFVAL(true));
+	ClassDB::bind_method(D_METHOD("cursor_set_line", "line", "adjust_viewport"), &TextEdit::cursor_set_line, DEFVAL(true));
 
 	ClassDB::bind_method(D_METHOD("cursor_get_column"), &TextEdit::cursor_get_column);
 	ClassDB::bind_method(D_METHOD("cursor_get_line"), &TextEdit::cursor_get_line);