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

Don't run formatter if file present in root #76

Closed
rawhat opened this issue Aug 21, 2023 · 9 comments
Closed

Don't run formatter if file present in root #76

rawhat opened this issue Aug 21, 2023 · 9 comments

Comments

@rawhat
Copy link

rawhat commented Aug 21, 2023

The find property is great for noting when to run a task. I see that there is also ignore_pattern for a particular file. But say I have two formatters, one of which I run conditionally if a file is present, and the other only if that first one is not matched, how can I do this with the current API? The original issue asking for the conditional with a function would cover both cases, where this is only an "if exists". Let me know if I'm missing something!

@barrett-ruth
Copy link
Contributor

@glepnir something to maybe consider. People want things to run in weird use cases. Turning a variable into a with a few lines is reasonable I think

@glepnir
Copy link
Member

glepnir commented Aug 22, 2023

maybe we can allow communication between multiple configurations.

@xiaoshihou514
Copy link
Collaborator

xiaoshihou514 commented Aug 22, 2023

or maybe provide api for writing unique formatting logics in the fn field? Like so:

ft("java"):fmt({
  fn = function(buf, range)
    if logic1 or logic2 then
      -- provided async wrapper for spawn
      require("guard.format").fmt_with(opts, buf, range)
    else
      ...
    end
  end
})

Then stacktracing may be a problem, but it will resolve similar issues like this without providing too many conditional apis.

@sudopseudocode
Copy link

I understand your ask for a function that lets you express both cases. I agree this could lead to cleaner configurations for complex use cases such as yours.

Regarding your ask for how to cover your use case with the current API, wouldn't you be able to successfully achieve this via:

local matched_file = 'some-file';

ft('lang'):fmt({
  -- First formatter
  ...
  find = { matched_file },
}):append({
  -- Second formatter
  ...
  ignore_pattern = { matched_file },
})

The second formatter isn't explicitly triggering because the first one didn't match, however, this is functionally the same because the find/ignore_pattern parameters are set to the opposite of each other. So functionally, the second formatter only runs when the first one doesn't run & vice versa.

Wouldn't this work? Or perhaps I'm missing something?

@xiaoshihou514
Copy link
Collaborator

@pauldiloreto ignore_pattern matches against the file you are formatting and not a root dir file

@sudopseudocode
Copy link

sudopseudocode commented Oct 10, 2023

@pauldiloreto ignore_pattern matches against the file you are formatting and not a root dir file

Got it, my mistake.

With that note, would it be sufficient to support glob patterns so we could pass a negative wildcard? Or perhaps just regex at that point?
Or would it be better to just have a function API like you mentioned earlier in this thread?

@xiaoshihou514
Copy link
Collaborator

I plan to do the api approach since it offers more flexibility (if no one is opposed to that)

@sudopseudocode
Copy link

I plan to do the api approach since it offers more flexibility (if no one is opposed to that)

Sounds good to me.
I just realized that this would also allow support for the use case of scanning a file (i.e. package.json) for certain keys and only running a formatter if some configuration is present there.

@barrett-ruth
Copy link
Contributor

I plan to do the api approach since it offers more flexibility (if no one is opposed to that)

sounds good to me

@xiaoshihou514 xiaoshihou514 mentioned this issue Jun 12, 2024
9 tasks
xiaoshihou514 added a commit that referenced this issue Jul 21, 2024
closes #68 #86 #123 #109 #76 #141 #111
## changes
- [x] remove uv.spawn wrapper in favour of vim.system
- [x] `do_lint` now respects *all* config fields
- [x] linter can now use lnum_end and col_end
- [x] added step by step tutorial for advanced usage
- [x] remove all version checking code, only supports 0.10+ from now on
- [x] apply exepath fix for windows

## internal changes
- [x] events now contains truly all autocmd related functions
- [x] utils now contains execution checking functions
- [x] use custom simpler table copy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants