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

Set prettier working directory to where .prettierignore is #3101

Merged
merged 4 commits into from
Aug 28, 2020

Conversation

iclanzan
Copy link
Contributor

Prettier does not use .prettierignore unless the current directory is the root where the .prettierignore file resides.

Prettier does not use `.prettierignore` unless the current directory is the root where the `.prettierignore` file resides.
@RyanSquared
Copy link
Member

related: #1095

@RyanSquared
Copy link
Member

I don't see the reason why --stdin-filename required CdString.

@stale
Copy link

stale bot commented Aug 13, 2020

This pull request has been automatically marked as stale because it has not been updated recently. Make sure to write tests and document your changes. See :help ale-dev for information on writing tests.
If your pull request is good to merge, bother w0rp or another maintainer again, and get them to merge it.

@stale stale bot added the stale PRs a bot will close automatically label Aug 13, 2020
@stale stale bot closed this Aug 15, 2020
@iclanzan
Copy link
Contributor Author

@w0rp do you think we can merge this?

@w0rp
Copy link
Member

w0rp commented Aug 17, 2020

I don't think it's the right solution. Unless the CWD is set to something, it could be anything at all. For many linters, the CWD doesn't matter. If the CWD has to be the same as the directory that .prettierignore is in, we should use the ale#path#FindNearestFile function to find it and set the CWD to that directory. That way the fixer will always use the right directory even when people have configured Vim to set the CWD to other directories.

I have Vim configured to set the CWD automatically to the directory the current file I'm editing is in.

@iclanzan
Copy link
Contributor Author

That wouldn’t work though because many projects don’t have a .prettierignore.

@w0rp
Copy link
Member

w0rp commented Aug 17, 2020

We'd handle the path possibly being there or not. Here's just one example of another tool doing a similar thing.

function! ale_linters#cpp#cquery#GetProjectRoot(buffer) abort
    " Try to find cquery configuration files first.
    let l:config = ale#path#FindNearestFile(a:buffer, '.cquery')

    if !empty(l:config)
        return fnamemodify(l:config, ':h')
    endif

    " Fall back on default project root detection.
    return ale#c#FindProjectRoot(a:buffer)
endfunction

@w0rp w0rp reopened this Aug 17, 2020
@stale stale bot removed the stale PRs a bot will close automatically label Aug 17, 2020
@w0rp
Copy link
Member

w0rp commented Aug 17, 2020

I've re-opened this. You can make the changes we need.

@iclanzan
Copy link
Contributor Author

Thanks for the guidance @w0rp, I think this is ready for another review!

@w0rp w0rp changed the title Don’t change directory for Prettier Set prettier working directory to where .prettierignore is Aug 28, 2020
@w0rp w0rp merged commit 80bd2e1 into dense-analysis:master Aug 28, 2020
@w0rp
Copy link
Member

w0rp commented Aug 28, 2020

Cheers! 🍻

jsit added a commit to jsit/ale that referenced this pull request Aug 28, 2020
* master: (195 commits)
  Close dense-analysis#3285 - lint_file is now dynamic
  Close dense-analysis#3309 - Add b:ale_lint_delay
  Fix dense-analysis#3323 - Set default for g:ale_filename_mappings
  Add sql-lint to supported tools
  dense-analysis#3324 - Enable rls by default
  Set prettier working directory to where .prettierignore is (dense-analysis#3101)
  Fix dense-analysis#3319 - Force modifications to buffers
  Fix dense-analysis#3318 - Escape macros when parsing C flags
  Fix C flag parsing and tests on Windows
  Mention --fast, and document running Windows tests locally
  dense-analysis#3318 Refactor C flag parsing to set up for quoting arguments
  dense-analysis#3266 - Catch echo visual selection errors
  Label the test cases more clearly
  Fix dense-analysis#3317 - Parse -include from C flags
  Fix dense-analysis#3316 - Repeat -relative for ALERepeatSelection
  Fix dense-analysis#3307 - Handle compile_commands paths better
  Fix a typo
  dense-analysis#3314 - Tell people how to make new plug mappings
  dense-analysis#3312 - Just check if additionalTextEdits is non-empty
  Fix dense-analysis#3312 - Fix a false positive for auto imports
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants