Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gui_filepicker: Show dialog on current project directory #472

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

kemzops
Copy link
Contributor

@kemzops kemzops commented Aug 3, 2024

#471 gui-filepicker:open-file and gui-filepicker:save-as

@kemzops kemzops changed the title gui_filepicker: Show dialog on current file's directory gui_filepicker: Show dialog on current project directory Aug 3, 2024
@hellium6
Copy link

Thanks for taking the time. Noticed 3 things:

  1. I think project_dir related code is being repeated. It would be better to keep it in a function and call it instead (diff below).

  2. Default directory detection has issues:

a) a file opened from project dir: works
b) a file opened from subdirectory from project dir: opens project dir, user has to navigate to subdirectory manually
c) a file opened from outside project dir: opens project dir, user has to navigate to specific dir manually
d) "unsaved" file: opens project dir, acceptable for me

Failure cases will cost the user more time to navigate to open & save.

I came up with these changes:

diff --git a/plugins/gui_filepicker.lua b/plugins/gui_filepicker.lua
index ef5ed78..26207ed 100644
--- a/plugins/gui_filepicker.lua
+++ b/plugins/gui_filepicker.lua
@@ -29,6 +29,16 @@ config.plugins.gui_filepicker = common.merge({
 	}
 }, config.plugins.gui_filepicker)
 
+local function get_project_dir()
+	local project_dir = core.project_dir
+	if project_dir and project_dir ~= "" then
+		project_dir = project_dir .. PATHSEP
+	else
+		project_dir = "~/"
+	end
+	return project_dir
+end
+
 local function dialog(kdialogFlags, zenityFlags, callback)
 	local utility = config.plugins.gui_filepicker.dialog_utility
 	local flags = zenityFlags -- The Default
@@ -68,14 +78,10 @@ local function dialog(kdialogFlags, zenityFlags, callback)
 	end)
 end
 
-command.add(nil, {
-	["gui-filepicker:open-file"] = function()
-		local project_dir = core.project_dir
-		if project_dir and project_dir ~= "" then
-			project_dir = project_dir .. PATHSEP
-		else
-			project_dir = "~/"
-		end
+command.add("core.docview", {
+	["gui-filepicker:open-file"] = function(dv)
+		local project_dir = get_project_dir()
+		local filepath = (dv.doc.abs_filename and dv.doc.abs_filename:match("(.*[/\\])") or project_dir)
 		
 		dialog(
 			{
@@ -84,7 +90,7 @@ command.add(nil, {
 			{
 				"--file-selection",
 				"--filename",
-				project_dir,
+				filepath,
 			},
 			function(abs_path)
 				core.root_view:open_doc(core.open_doc(common.home_expand(abs_path)))
@@ -151,14 +157,10 @@ command.add(nil, {
 
 command.add("core.docview", {
 	["gui-filepicker:save-as"] = function(dv)
-		local project_dir = core.project_dir
-		if project_dir and project_dir ~= "" then
-			project_dir = project_dir .. PATHSEP
-		else
-			project_dir = "~/"
-		end
-		
+		local project_dir = get_project_dir()
 		local filename = dv.doc.filename or "new_file"
+		filename = (dv.doc.abs_filename and dv.doc.abs_filename or project_dir .. filename)
+
 		dialog(
 			{
 				"--getsavefilename",
@@ -167,7 +169,7 @@ command.add("core.docview", {
 				"--file-selection",
 				"--save",
 				"--filename",
-				project_dir .. filename,
+				filename,
 			},
 			function(abs_path)
 				dv.doc:save(abs_path, abs_path)
  1. I'm not sure about:
project_dir = "~/"

I think for *nix OSs, getting the value of $HOME would be better.

Not sure about windows, I don't use it. There seems to be zenity “compatible” binaries for Windows available. Perhaps env var USERPROFILE should be used?

project_dir = os.getenv("HOME") or os.getenv("USERPROFILE") or "~/"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants