search

savi-lang / savi

A fast language for programmers who are passionate about their craft.
BSD 3-Clause "New" or "Revised" License
154 stars 12 forks source link

Expose `dirname` from `SourceCodePos` #397

Closed mneumann closed 1 year ago

mneumann commented 1 year ago

This will enable Spec to print absolute or project-root relative file paths.

mneumann commented 1 year ago

@jemc For Spec: I'd not like to report the full absolute path of the assert that was failing. Instead I would like to report the path relative to the project root, e.g. src/Spec.savi. Getting this information at runtime is tricky. We don't have pwd/getcwd in Savi yet and even then the question remains, do we report relative to the current working directory or relative to the executing bin/spec .... So, in addition to the absolute dirname, I'd expose the directory relative to the root manifest.

Any suggestions?

mneumann commented 1 year ago

Mainly, reporting absolute paths in Spec will break the Spec for Spec, as it would include my absolute paths. Also, I never really liked it to expose the absolute paths of my failing tests in "bug reports". No one should see something like /home/foobar/Clients/StupidClientA/... if I decide to name a client like this (which I don't) and reflect this in my directory structure :)

jemc commented 1 year ago

I agree that having Spec show the relative path compared to the path of the root manifest would be the most correct behavior - I prefer this over something more magical involving the "current working directory".

jemc commented 1 year ago

@mneumann - I think ctx.root_package_link.path will get you the path to the root package's manifest directory.

mneumann commented 1 year ago

@jemc relative to root package might not always work in case the packages would not reside under deps. In case of Spec I would report pkgname + package relative file path.

jemc commented 1 year ago

relative to root package might not always work in case the packages would not reside under deps. In case of Spec I would report pkgname + package relative file path.

I'd honestly still prefer them being relative to the root package, so in the case of non-root package source files I'd be able to see which package they came from. But I'm not strongly opinionated enough to argue about it, as Savi's name requirements make collisions a lot less likely.

In the strange case you raise of dependencies not being in deps (which would be true of core until we do #366, so I guess it's not that strange at the moment), I'd expect to see ../ segments added to the head of the path until reaching a common parent directory that holds both the root package and the out-of-source-tree package.

mneumann commented 1 year ago

@jemc I agree with you. Added filepath_rootpkgrel. We should change the naming. But this works fine so far with Spec. So I am going to merge and feel free to fixup later on.