workspace::Open: Add trailing / to directories on completion

[5brian]

Problem: It takes two keys to view subdirectories in Zed's open workspace prompt. (tab to complete and / )
Fix: Append / to directories in the Zed workspace::Open.

To reproduce: mod-o or workspace::Open with "use_system_path_prompts": false.
Right now, using Zed's open workspace prompt instead of the system prompt requires a / input after every tab complete, if trying to navigate into subdirectories multiple folders deep from home folder.

For example:
Trying to open foo/bar/folder, home directory is foo

Previous path autocompletes: foo/bar

The prompt does not show subdirectories in bar yet. Still needs an / input afterwards.

New changes: Checks if path is a directory and appends / to the completion if so.

Results in seeing the subdirectories right after autocomplete.

Helps save a few keys but not that big of a deal overall, looks like it wont work since windows uses backslash?

Zsh:

Zsh does it too (with oh my zsh i think). I didn't add it to the render_matches so Zed's prompt would still look the same as before.

Release Notes:

• N/A

[zed-industries-bot]

	Warnings
⚠️	This PR is missing release notes. Please add a "Release Notes" section that describes the change: Release Notes: - Added/Fixed/Improved ... If your change is not user-facing, you can use "N/A" for the entry: Release Notes: - N/A

Generated by 🚫 dangerJS against a8d8141

[SomeoneToIgnore]

Can you elaborate why is it needed and what problem does this PR try to solve?
Maybe, there's a way to create a test to illustrate this?

If I understand correctly, file finder can only show files in the results:

those do not need any trailing /.
Directories that correspond to files have the trailing path separator already.

Last but not least, this change does not make any sense on Windows as a different kind of a separator has to be used there.

[5brian]

@SomeoneToIgnore Sorry for the poor explanation, i just updated the description. I was referring to Zed's open workspace prompt (workspace::Open with "use_system_path_prompts": false in settings) instead of the file finder.

I was worried about windows backslash too, but it looks like most of the code uses / as well. But it is looking like windows is failing so i am probably wrong.

zed/crates/file_finder/src/open_path_prompt.rs

Lines 96 to 97 in 5f1f3f1

	// todo(windows)
	// Is this method woring correctly on Windows? This method uses `/` for path separator.

[SomeoneToIgnore]

Thank you, this is much more clear now, I have forgotten entirely about this corner.

It has issues with Windows indeed, as

zed/crates/file_finder/src/open_path_prompt.rs

Lines 96 to 97 in adb0d32

	// todo(windows)
	// Is this method woring correctly on Windows? This method uses `/` for path separator.

notes, placeholder_text, confirm_completion and update_matches use / (and some other Zed corners, I bet).

This results in the quite odd path handling:

windows.mp4

---

Given the context, the PR is a nice improvement indeed.

• Let's use std::path::MAIN_SEPARATOR instead of / in the related code as we're touching it.

• Not insisting on this, but I also think it should be relatively straightforward to copy a test like

zed/crates/file_finder/src/file_finder_tests.rs

Lines 149 to 185 in adb0d32

	#[gpui::test]
	async fn test_complex_path(cx: &mut TestAppContext) {
	let app_state = init_test(cx);
	app_state
	.fs
	.as_fake()
	.insert_tree(
	path!("/root"),
	json!({
	"其他": {
	"S数据表格": {
	"task.xlsx": "some content",
	},
	}
	}),
	)
	.await;
	let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
	let (picker, workspace, cx) = build_find_picker(project, cx);
	cx.simulate_input("t");
	picker.update(cx, |picker, _| {
	assert_eq!(picker.delegate.matches.len(), 1);
	assert_eq!(
	collect_search_matches(picker).search_paths_only(),
	vec![PathBuf::from("其他/S数据表格/task.xlsx")],
	)
	});
	cx.dispatch_action(SelectNext);
	cx.dispatch_action(Confirm);
	cx.read(|cx| {
	let active_editor = workspace.read(cx).active_item_as::<Editor>(cx).unwrap();
	assert_eq!(active_editor.read(cx).title(cx), "task.xlsx");
	});
	}

and adjust it to check that we auto add / to directories — this way, we'll ensure Windows is not broken in this corner.

Outdated

[5brian]

I see, looks like the / broke on windows. Thanks for the review! I'll try to get to this this weekend.