Keep and document implicit cd? The / (division command) invoked with no args does cd /

[krader1961]

While chipping away at issue #1062 to improve unit test coverage I noticed this block of code:

elvish/pkg/eval/builtin_fn_num.go

Lines 281 to 289 in 3300c81

	func slash(fm *Frame, args ...float64) error {
	if len(args) == 0 {
	// cd /
	return fm.Chdir("/")
	}
	// Division
	divide(fm, args[0], args[1:]...)
	return nil
	}

I noticed it because no unit test exercises the return fm.Chdir("/") statement and I was surprised that module did not have 100% coverage.

I could add a test for that case but I think the feature should be removed. Note that the only place I can find that documents the "implicit cd" feature is for the / command.

Note that the current behavior can result in silent bugs:

> pwd
/Users/krader
> l = []
> / $@l
> pwd
/

That chdir should not have occurred.

[zzamboni]

I never use implicit cd myself. In any case, it seems like something that should only be used in interactive mode - maybe it should be a feature of the editor and not of the language itself?

[xiaq]

Yes ideally this should be a feature of the editor not the language itself. However, implicit cd is a feature that is hard to separate from the language.

What do y'all think about letting the location mode accept literal paths? The logic can work like:

• If the user enters a path that starts with /, ./ or ../ and is a valid directory path, the location mode will always offer the literal directory as the first candidate

• Otherwise, the location mode searches through the directory history.

A small downside is that tab completion is lost - but in that case the user could revert to using cd, which is hopefully not too bad.

[krader1961]

What do y'all think about letting the location mode accept literal paths?

That's preferable to the current behavior but I'm not sure it's worth saving one to three keystrokes. Especially since it doesn't support tab completion.

Consider the scenario where CWD is $HOME and I have a tmp subdir. Today I can't type t and press tab to expand to tmp. I have to type ./t then tab. That saved me exactly one keystroke. Whereas I can type cd t then tab to expand the buffer to cd tmp/.

[krader1961]

There was a recent discussion, in the context of issue #1170, that Navigation mode does two things:

1. change the CWD, and
2. insert filenames on the command line.

Location mode only changes the CWD. Ideally only Location mode would support changing the CWD while melding the current behavior (selecting an item from the CWD history) and Navigation mode. Then Navigation mode would only be used for selecting filenames, not changing the CWD. I have no idea what such a UI would look like. But I'm extremely unhappy that using Navigation mode to insert filenames on the command line also, as a side-effect, changes the CWD.

[xiaq]

@krader1961

That's preferable to the current behavior but I'm not sure it's worth saving one to three keystrokes. Especially since it doesn't support tab completion.

That is a good point.

Today I can't type t and press tab to expand to tmp. I have to type ./t then tab. That saved me exactly one keystroke.

That's a fair point; it only saves 1 keystroke in this example, but for ../foo and /foo it does save 3 keystrokes. I am personally quite used to implicit cd so I will fight for every keystroke :)

But I'm extremely unhappy that using Navigation mode to insert filenames on the command line also, as a side-effect, changes the CWD.

I am not convinced that the navigation mode has a broken operation model - it might help a bit if you think of the primary purpose of the navigation mode as, well, navigating in the filesystem, and the functionality to insert filename as a secondary functionality. Feel free to open another issue if you'd like to discuss this further.

[krader1961]

That's a fair point; it only saves 1 keystroke in this example, but for ../foo and /foo it does save 3 keystrokes. I am personally quite used to implicit cd so I will fight for every keystroke :)

I would agree if we were talking about how people interacted with computers more than two decades ago. Which includes me since I started programming in high-school in 1977. Which is why so many UNIX commands are cryptic. Nonetheless, directory changes, at least in a particular session for me, is rare enough that eliding the cd prefix (or equivalent magic sequences that invoke things like Elvish location mode) isn't worth the complexity.

[krader1961]

I still feel, quite strongly, that implicit cd has more problems than the value of eliding three keystrokes in situations most users will never care about. The fact this feature is only documented in the / command seems like another strong argument that it is hard to document and discover by new users. Even if it was documented in the section of the cookbook that talks about "UI recipes" I would argue it is "too magical".

If nothing else consider that the implicit "cd" string is recorded in the interactive command history without a "cd " prefix. So if I start a new Elvish shell and select that "command" it might change to that directory or might run a command of the same name. Ugh!

[krader1961]

It looks like I might have misunderstood an earlier comment by @xiaq regarding location mode. I agree that augmenting location mode to special-case paths such as ./x and ../y is a good idea. Using it requires only one additional character (whatever enables location mode) compared to the current implicit cd behavior, is arguably more consistent with user expectations, and is less likely to result in unexpected behavior (i.e., bugs) since a feature intended to be used only at an interactive prompt should not be usable in a non-interactive Elvish program.

Also, I should have noted earlier that the implicit cd behavior isn't implemented solely by the / (division) command. It also affects resolution of external "commands":

elvish/pkg/eval/external_cmd.go

Lines 55 to 68 in 4717deb

	func (e externalCmd) Call(fm *Frame, argVals []interface{}, opts map[string]interface{}) error {
	if len(opts) > 0 {
	return ErrExternalCmdOpts
	}
	if fsutil.DontSearch(e.Name) {
	stat, err := os.Stat(e.Name)
	if err == nil && stat.IsDir() {
	// implicit cd
	if len(argVals) > 0 {
	return ErrImplicitCdNoArg
	}
	return fm.Evaler.Chdir(e.Name)
	}
	}

Note that fsutil.DontSearch() special-cases .. but not ., which is unexpected since I would expect that if .. is equivalent to cd .. then . would be equivalent to cd .:

elvish/pkg/fsutil/search.go

Lines 14 to 17 in 4717deb

	func DontSearch(exe string) bool {
	return exe == ".." || strings.ContainsRune(exe, filepath.Separator) ||
	strings.ContainsRune(exe, '/')
	}

[krader1961]

... since a feature intended to be used only at an interactive prompt should not be usable in a non-interactive Elvish program.

In case the implications of that statement weren't obvious create and run the following Elvish program:

#!/usr/bin/env elvish
pwd
..
pwd

I can't see a good reason to support that even if you ignore the fact that the UNIX meaning for .. as a path component is not universal. It looks like something that is too likely to be abused and thus be a security problem.

[krader1961]

Having read this issue again, more than a year after my last comment, I am still convinced that the implicit cd feature should be eliminated. This feature has too much of a "spooky action at a distance" feel to it. Not to mention all of the other problems documented in previous comments such as the fact this "feature" works in a non-interactive context which is going to be surprising to most users and possibly a security problem.

[krader1961]

Searching for commands that mention exactness-preserving reminded me of this issue. Here are more problematic examples:

elvish -c '/'
elvish -c '/tmp'
elvish -c '..'

Most users will ask why those seemingly do nothing rather than throw an exception since they are not valid Elvish statements. The implicit cd behavior should be removed because it is magical and affects non-interactive (i.e., non-REPL) use cases. Leading to subtle bugs. This behavior should be moved into location mode if retained at all, but should definitely be removed from the core language behavior.

[krader1961]

Not to mention that invalid paths do not perform an implicit cd; they result in a somewhat confusing exception:

elvish> > elvish -c '../argle-bargle'
Exception: exec: "../argle-bargle": stat ../argle-bargle: no such file or directory
code from -c:1:1: ../argle-bargle
Exception: elvish exited with 2

Compare that with this example:

elvish> elvish -c 'argle-bargle'
Exception: exec: "argle-bargle": executable file not found in $PATH
code from -c:1:1: argle-bargle
Exception: elvish exited with 2

I don't see a good reason for the two error messages to be different. Yes, the former is an explicit path that bypasses searching the $E:PATH var for the command while the latter does not. But the error message for the former doesn't make it obvious that an implicit cd was attempted. This confusion is eliminated if the implicit cd feature is moved into the explicitly interactive location mode.

Also, that the first example says "Exception: exec:" is questionable since it was also attempting an implicit cd operation and only fell back to trying to execute that path when the implicit cd failed. Which is another reason why the two behaviors should not be intertwined.

[xiaq]

Removing implicit cd from the language sounds fine as long as location mode gains the functionality of doing a literal chdir.

[krader1961]

Enhancing location mode to support a filter that puts the filter string at the top of the directory list if it corresponds to a literal directory is trivial. The hardest part of this change is an appropriate unit test. The second hardest aspect is the documentation. So I'll tackle this. I should have a pull-request in a day or two.

[krader1961]

LOL! Implementing this, including a relevant unit test on Unix platforms, is straightforward. Testing it on Windows is more difficult.