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

Output relative paths rather than absolute paths #13376

Closed
Hubro opened this issue May 30, 2020 · 9 comments
Closed

Output relative paths rather than absolute paths #13376

Hubro opened this issue May 30, 2020 · 9 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Comments

@Hubro
Copy link

Hubro commented May 30, 2020

The version of ESLint you are using.
v7.1.0

The problem you want to solve.
Currently when I run eslint on a directory, it groups the errors by file, and each group has the absolute path to the file at the top.

I would like to request that eslint prints the relative path to the file instead.

For example, when I run eslint --ext ts,tsx src/ in my project root, I get several blocks like this:

/home/hubro/Dropbox/Company Foo Projects/Some Category/my-project-name/my-project-name-client/src/some-subdir/foo/bar/relay-env.ts
  17:3   warning  'cacheConfig' is defined but never used  @typescript-eslint/no-unused-vars
  18:3   warning  'uploadables' is defined but never used  @typescript-eslint/no-unused-vars
  57:16  warning  Missing return type on function          @typescript-eslint/expl

First of all the path is needlessly long, but more problematically the full path contains spaces, so VSCode fails to open the path using Ctrl+Click. This means I have to manually browse to each file with errors whenever I run eslint, which is frustrating.

I even tried to avoid the space issue by symlinking the project folder under ~/projects but it doesn't matter, eslint apparently resolves the absolute path to the file anyway. This also causes the additional issue that even if VSCode could now open the path, I would now be opening the same files with 2 separate paths, meaning they would show up twice in my recent files list, which is also frustrating.

Your take on the correct solution to problem.
I would like to configure eslint so it outputs something like this instead:

src/some-subdir/foo/bar/relay-env.ts
  17:3   warning  'cacheConfig' is defined but never used  @typescript-eslint/no-unused-vars
  18:3   warning  'uploadables' is defined but never used  @typescript-eslint/no-unused-vars
  57:16  warning  Missing return type on function          @typescript-eslint/expl

Or at the very least I would like to stop it from resolving links into absolute paths.

Is there any way this can be done currently? Would it be reasonable to add a setting to control this?

Are you willing to submit a pull request to implement this change?
If given some pointers to where to start, sure.

@Hubro Hubro added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels May 30, 2020
@kaicataldo
Copy link
Member

This seems like a bug report/feature request for VS Code.

Changing the default formatter for this specific case feels a bit like a kludge to me. I think we should stick with absolute paths in the default formatter because there's no ambiguity around which file the result is referring to, and we should always be striving for less ambiguity and less magic in the default happy path.

ESLint allows for user-defined custom formatters. I would suggest taking the existing default formatter and tweaking it to your liking.

@kaicataldo kaicataldo added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels May 30, 2020
@Hubro
Copy link
Author

Hubro commented Jun 1, 2020

Yes I wasn't suggesting anything should change by default, I was rather hoping that a setting existed that could affect this.

After quickly skimming the custom formatters page you linked, it seems like options can be added to formatters through environment variables. Would you be opposed to add an option like STYLISH_PRINT_RELATIVE_PATHS=1 to make the stylish formatter resolve relative paths rather than print the absolute ones?

@kaicataldo
Copy link
Member

I personally don't think we should be changing existing formatters at this point, particularly since custom formatters are already supported. Let's see what the rest of the team thinks.

@mysticatea
Copy link
Member

Actually, custom formatter cannot convert paths to relative because formatters cannot know the current working directory (cwd option of CLIEngine/ESLint classes). They can use process.cwd(), but I'm not sure if it's safe.

I wonder if we should support to access cwd from formatters.

@Hubro
Copy link
Author

Hubro commented Jun 2, 2020

In what scenario would it not be safe to use process.env.PWD? (Is there any practical difference between that and process.cwd()?)

I mean, if the user explicitly clears the environment before calling ESLint it would be unavailable:

~ 
➜ echo 'console.log(process.env.PWD)' | env -i node
undefined

But all shells (afaik) will always make PWD available to subprocesses, even when called with an empty environment and without any initialization scripts:

~ 
➜ env -i bash --noprofile --norc -c 'env'
PWD=/home/hubro
SHLVL=0
_=/usr/bin/env

~ 
➜ env -i zsh -d -c 'env'
HOME=/home/hubro
LOGNAME=hubro
SHLVL=0
PWD=/home/hubro
OLDPWD=/home/hubro
_=/bin/env

In any case it's a simple check in the formatter to run if (process.env.PWD === undefined) ... and raise an error message, if the user requested to get relative paths. This should basically never be an issue unless the user explicitly clears their environment before calling eslint.

@kaicataldo
Copy link
Member

I wonder if we should support to access cwd from formatters.

Ah, I didn't know this. That seems like information a formatter should have, given that filepaths are such an important part of reporting results.

@mysticatea
Copy link
Member

@Hubro Our Node.js API has cwd option and users can give arbitrary path as cwd. ESLint uses the given cwd at all. But, currently, formatters cannot access that cwd. So we should support to access cwd from formatters.

@kaicataldo I will write an RFC for that.

@mysticatea
Copy link
Member

I have wrote:

@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Jul 8, 2020
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jan 5, 2021
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jan 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

No branches or pull requests

3 participants