tools icon indicating copy to clipboard operation
tools copied to clipboard

Add .gitignore support

Open NicholasLYang opened this issue 2 years ago • 16 comments

Currently we have hard coded folders that we ignore. We should change that to follow the .gitignore

NicholasLYang avatar Apr 25 '22 17:04 NicholasLYang

What are the requirements of .gitignore? Is it necessary?

My concern is that we should not ignore files that are ignored by the VCS. Mainly because Rome is not a VCS and there's value in processing files that are ignored.

It can provide value for operations that involve VCS, so we should carefully understand the when and how.

ematipico avatar Apr 26 '22 07:04 ematipico

IMO we should revisit this along with #2493. It's an extremely common use case and has prior art in ripgrep and fd.

NicholasLYang avatar May 23 '22 21:05 NicholasLYang

Should we add support to of .gitignore file only when we are inside a project that has our config file? (rome.json)

We don't have support of a configuration file yet, but once we do, we could add support for .gitignore too.

ematipico avatar May 24 '22 08:05 ematipico

Would that be the ideal setup? I'm not a huge fan of dprint requiring a config file to use the formatter. While a rome.json would not be mandatory, I anticipate that the majority of people will want to run the formatter on only files that are checked into git

NicholasLYang avatar May 24 '22 14:05 NicholasLYang

What if:

  • you have rome.json, then the ignored files will go inside the relative ignore section (as said before, there's value in processing files that are ignored inside a VCS);
  • you don't have rome.json, then we use .gitignore, with all the ups and downs that come with it;

ematipico avatar May 24 '22 15:05 ematipico

That could work, although we'd have to figure out a way to make that clear to users. It might be very confusing to have Rome obey the .gitignore, then when a rome.json file is added, suddenly it ignores the .gitignore.

NicholasLYang avatar May 24 '22 20:05 NicholasLYang

IMO we should revisit this along with #2493. It's an extremely common use case and has prior art in ripgrep and fd.

I've been thinking more about this, and I am more convinced that we should not supported this out of the box (maybe behind flag). The examples you mentioned are different from us (they don't write files, for example). If you have examples of CLIs similar to Rome (formatter, bundler, test framework, linter, etc.), that would build your case.

Still, I think it should an opt in feature, if we really want to support it.

ematipico avatar May 27 '22 13:05 ematipico

Closing, ignored files will be part of the configuration file

ematipico avatar Aug 03 '22 09:08 ematipico

Hi, just wanted to voice my personal experience. I just downloaded rome to try it out, and was surprised when it tried to format files that are in my .gitignore. I find it mildly annoying that I need to duplicate my .gitignore in my rome config. Feel free to ignore me, I'm just providing a bit of unsolicited feedback. :)

IanVS avatar Nov 09 '22 21:11 IanVS

@IanVS Yeah this is good feedback. I think we should likely consider doing this.

sebmck avatar Nov 09 '22 21:11 sebmck