TMSU icon indicating copy to clipboard operation
TMSU copied to clipboard

Why are relative paths handled like that in output?

Open ipkiss42 opened this issue 3 years ago • 4 comments

The handling of paths in several subcommands is quite surprising, because it hard-codes ./ and ../ prefixes.

# Setup the test directory
$ mkdir -p /tmp/test
$ cd /tmp/test
$ touch foo

# Create some tags
$ tmsu init
tmsu: /tmp/test: creating database
$ tmsu tag foo foo
tmsu: new tag 'foo'
$ tmsu tag --force ../parent parent
tmsu: new tag 'parent'
$ tmsu tag --force ../../grand-parent grand-parent
tmsu: new tag 'grand-parent'

# Example of the `files` subcommand: file paths have a `./` or `../` prefix, or are absolute
$ tmsu files
../parent
./foo
/grand-parent
# Even more confusing?
$ (mkdir dir && cd dir && tmsu files)      
/tmp/parent
../foo
/grand-parent

As a user, I find this behaviour confusing :)

Questions:

  • Is there any added value in hard-coding the ./ prefix in the output? It is already implicit, and I can't think of any other CLI tool doing that by default.
  • What about removing the special handling for ..? It seems arbitrary (why not do it for ../.. as well) and is also not usual for CLI tools. So files in the parent folder could be printed with their absolute path.

ipkiss42 avatar Sep 20 '20 17:09 ipkiss42

Is there any added value in hard-coding the ./ prefix in the output? It is already implicit, and I can't think of any other CLI tool doing that by default.

I add this to be more explicit with the paths printed out. Without a prefix it is unclear where the path is rooted: is it rooted at the working directory or at the database root? If you've ever used the find command, you'll see it does the same thing.

What about removing the special handling for ..? It seems arbitrary (why not do it for ../.. as well) and is also not usual for CLI tools. So files in the parent folder could be printed with their absolute path.

I wanted the paths to be 'friendly', as in relative, but without an unreadable number of parent directory (..) elements. Historically the default behaivour was to have a TMSU database in your home directory and tag files anywhere on your filesystem. With the introduction of init, the model has changed to be a bit more like Git or Mercurial, where you initialise a place where you have files. But it's still possible to tag files outside of that place, and it's still possible to explicitly use a database in a different location. So then I had to make a decision on how to show paths:

  1. Always show them relative to the database
  2. Always show them relative to the working directory
  3. Always show them absolute
  4. Some hybrid

I didn't like the absolute option, as it's too verbose to be readable. Always showing them relative to the working directory meant you have long strings of .. sometimes, which I also didn't like. Relative to the database is just confusing: even Git doesn't do this.

So we have a hybrid where it'll use relative paths for those files that are below the working directory or in the immediate parent, but otherwise switch to absolute paths for those that would be too long-winded.

I agree it can probably be simplified. I would imagine always showing paths relative to the working directory (like Git) would make the most sense (though this might cause a problem in Windows, where you have multiple roots). I'll add this to 0.8.

oniony avatar Sep 21 '20 11:09 oniony

I add this to be more explicit with the paths printed out. Without a prefix it is unclear where the path is rooted: is it rooted at the working directory or at the database root? If you've ever used the find command, you'll see it does the same thing.

Actually the find command echoes its input:

$ find src/entities 
src/entities
src/entities/settings.rs
$ find ./src/entities
./src/entities
./src/entities/settings.rs
$ find ~/dev/tmsu-rust/src/entities 
/home/ipkiss/dev/tmsu-rust/src/entities
/home/ipkiss/dev/tmsu-rust/src/entities/settings.rs

In this sense it is very similar to the tags subcommand:

$ tmsu tags file1 src/../file1 
file1: t1 t2
src/../file1: t1 t2

I didn't like the absolute option, as it's too verbose to be readable. Always showing them relative to the working directory meant you have long strings of .. sometimes, which I also didn't like. Relative to the database is just confusing: even Git doesn't do this.

It actually does with git ls-files but not with git status (unless using the git -C option). Go figure... :)

I agree it can probably be simplified. I would imagine always showing paths relative to the working directory (like Git) would make the most sense (though this might cause a problem in Windows, where you have multiple roots). I'll add this to 0.8.

Git does not deal with absolute paths, so it may not be the best comparison.

I agree about not using absolute paths everywhere, but relative paths everywhere are arguably even worse: after two or three .., I find it hard to reason about where the files are.

That would live us with option 4 (hybrid). There are still three main variants:

  1. Paths within $CWD/.. are relative, other paths are absolute. This is the current implementation.
  2. Paths outside the root are absolute, paths within are relative to the root (like git ls-files). This allows a user to easily spot files which are outside the database (see also #207).
  3. Paths outside the root are absolute, paths within are relative to the current working directory (like git status). This is maybe more intuitive, and it limits the risk of having too many .. components in paths.

I personally think that variants 2 and 3 both have their merits. Maybe we could use a new flag (tmsu files --relative=[root|cwd]) to let users choose which of the two they prefer?

ipkiss42 avatar Sep 21 '20 13:09 ipkiss42

Of your suggestions option 3 is probably the most palatable to me.

I'm not a fan of little used options: it just adds more scope for things to go wrong, more complexity, &c.

oniony avatar Sep 21 '20 13:09 oniony

I'm fine with starting with option 3 and re-assessing if there is a need for something else after that.

ipkiss42 avatar Sep 21 '20 14:09 ipkiss42