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

Detect paths and highlight them if missing #2907

Open
Myzel394 opened this issue Mar 20, 2024 · 4 comments
Open

Detect paths and highlight them if missing #2907

Myzel394 opened this issue Mar 20, 2024 · 4 comments
Labels
feature-request New feature or request

Comments

@Myzel394
Copy link

Often when dealing with config files, a lot of paths are used. Being able to easily detect those paths when highlighted and see if they are missing would be such as live saver.

I propose to add an option --show-paths with the options:

  • --show-paths=missing, which will only highlight missing paths
  • --show-paths or --show-paths=all, which will highlight existing paths in green and non-existing paths in red (or in different colors dependent on the theme)

To detect paths I'd use a simple regex.

I can open a PR if you're okay with this idea and this implementation. I haven't looked much into the code, but I know a bit about Rust.

@Myzel394 Myzel394 added the feature-request New feature or request label Mar 20, 2024
@keith-hall
Copy link
Collaborator

interesting idea. I have a few concerns about the idea which I want to share in case we can somehow address them:

  • I would worry that many paths are relative paths and there would be no easy way to check that they exist or not, especially when piping stdin.
  • Would it somehow be smart enough to ignore relative urls so there aren't a lot of false negatives? (i.e. most unix-like paths start with /lib, /home/, /etc/ , /var/ that sort of thing, and could perhaps be "hardcoded"?)
  • Would it cater for Windows paths as well?
  • It would have to be a post-processing step, which could slow bat down (running the regex patterns and checking whether the paths exist could be fairly expensive operations?) and complicate the codebase. (Likely, the slowness is mitigated by having it only execute when the command line argument is supplied, as it wouldn't be enabled by default.)
  • It could conflict with the syntax highlighting - I'm imagining JSON strings where \t represents a tab etc. which could be fun for Windows paths like .\tests\...

@Myzel394
Copy link
Author

I would worry that many paths are relative paths and there would be no easy way to check that they exist or not

If a path is relative, shouldn't it be relative to the file then? In this case we can check for files relative to the given file and check if those paths exist or not

especially when piping stdin.

Do you mean something like: cat /folder/test.txt | bat (bad example, but you get the idea :D) - hmmm, in this case this wouldn't be easy. Maybe we just start with absolute paths or only highlighting existing paths, or disable paths at all for piped stdin

Would it somehow be smart enough to ignore relative urls so there aren't a lot of false negatives? (i.e. most unix-like paths start with /lib, /home/, /etc/ , /var/ that sort of thing, and could perhaps be "hardcoded"?)

I'm very against hardcoding (especially since the reason why I initially opened this feature request was that I wanted my /media folders to be checked :D). This is another point that would speak for only highlighting existing paths

Would it cater for Windows paths as well?

I don't really use Windows myself, but I know that Windows' path just differ from starting with something like C: or D: and using the escape character instead of the backslash - building an regex for that would be as trivial as building one for unix.

It would have to be a post-processing step, which could slow bat down (running the regex patterns and checking whether the paths exist could be fairly expensive operations?)

I haven't looked into the codebase yet, so you know more about I do here :D However, if we just parse the currently visible paged content, that will make stuff faster. Also, regarding:

I'm imagining JSON strings where \t represents a tab etc

to avoid this we simply cache the files existence, this way the tab character will only be checked once.

It could conflict with the syntax highlighting

why?

which could be fun for Windows paths like .\tests...

I don't quite get your point here - could you elaborate?


So based on this, I'd propose we only check for existing paths for the first iteration. Also always check relatively to the file. When some content is passed through stdin, let's just check relative to cwd.

What do you think about this?

@keith-hall
Copy link
Collaborator

I'm imagining JSON strings where \t represents a tab etc

to avoid this we simply cache the files existence, this way the tab character will only be checked once.

It could conflict with the syntax highlighting

why?

which could be fun for Windows paths like .\tests...

I don't quite get your point here - could you elaborate?

imagine some JSON like:

{
  "test": ".\test"
}

it might at first glance look like a relative Windows path, but the \t will initially be correctly syntax highlighted as a character escape. So further highlighting it as a path would presumably override that and - if it is meant to be a valid path, make the mistake harder to spot because the literal path .\test could exist. And having special logic depending on the type of file, the position of the path (inside a string etc where different escaping rules apply) in the file sounds like a can of worms I don't want to open ;)

@Myzel394
Copy link
Author

Myzel394 commented Mar 26, 2024

{
  "test": ".\test"
}

Wouldn't you use:

{
  "test": ".\\test"
}

instead?

I think those cases are quite rare and most of the time we will be able to detect them correctly.

I'll take a look at the code when I got some free time and will then open a PR. I think this feature would be much more helpful than it could harm, especially since it will be opt-in.

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

No branches or pull requests

2 participants