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

Add --ignore <glob> parameter #1095

Closed
sdavids opened this issue Sep 20, 2024 · 7 comments
Closed

Add --ignore <glob> parameter #1095

sdavids opened this issue Sep 20, 2024 · 7 comments

Comments

@sdavids
Copy link
Contributor

sdavids commented Sep 20, 2024

Having --ignore <glob> or however you want to call it would improve the usability of this tool.

Yes, there are workarounds but they have issues.

The non-standard key in .editorconfig has its issue, too.


Filtering while tree-walking inside of the tool is always better than piping to xargs, doing a for-loop, or having to create an extra file.

SC2038
SC2044

@mvdan
Copy link
Owner

mvdan commented Sep 20, 2024

What exactly is the problem with the entry in editorconfig? If the issue with the key is the lack of the prefix, we can fix that. At this point it's still unlikely I will add a flag for this unless good arguments are brought forward. "I find a flag easier than an editorconfig file" doesn't seem like a particularly strong argument.

@sdavids
Copy link
Contributor Author

sdavids commented Sep 20, 2024

It is not "easier" it is not having the file at all.


Let's say you want to format a repository but the maintainer of that project does not want the .editorconfig file committed.

You then have to have to juggle with git stash and/or always be careful not to commit .editorconfig.

Or remember to:

$ printf '[node_modules/**]\nignore = true\n' >.editorconfig && shfmt -w . && rm .editorconfig

every time you work on that repo.

Instead of

$ shfmt -w --ignore node_modules .

@mvdan
Copy link
Owner

mvdan commented Sep 20, 2024

It seems to me like you have manual steps no matter what you do in this scenario. Yes, the flag is shorter/faster than the other methods, but it's still a manual step you must remember.

You could always drop an .editorconfig in a parent directory. The search for those files does not end at the root of the VCS repository. You could, for example, have an editorconfig file to ignore all node_modules directories somewhere closer to your home directory.

Also note that editorconfig entries are a list of patterns to match filenames. And we also support matching by language (#664 (comment)). Fitting all this into a flag seems odd.

@sdavids sdavids changed the title Add --exclude parameter Add --exclude <glob> parameter Sep 20, 2024
@sdavids
Copy link
Contributor Author

sdavids commented Sep 20, 2024

I think the most common use case is that you format a directory and find out that 1 or 2 files (or 1 directory containing files) were formatted even though you did not want them to be formatted.

$ shfmt -w .
$ git status -s
 M node_modules/test.sh
 M test.sh

😲

$ git reset --hard
$ shfmt --help

$ shfmt -w --ignore node_modules .
$ git status -s
 M test.sh
$ git add -A
$ git commit -m 'chore: format'

🎉


$ shfmt -w .
$ git status -s
 M node_modules/test.sh
 M test.sh

😲

$ git reset --hard
$ shfmt --help

🤔

$ man shfmt

📘👀⌛

$ printf '[node_modules/**]\nignore = true\n' >.editorconfig
$ shfmt -w .
$ git status -s
 M test.sh
?? .editorconfig

🤔

🌐👀⌛

$ git add -A -- ':!.editorconfig'
$ git commit -m 'chore: format'
$ git stash push -u -m shfm_exclude_node_modules

🎉


You could always drop an .editorconfig in a parent directory. The search for those files does not end at the root of the VCS repository. You could, for example, have an editorconfig file to ignore all node_modules directories somewhere closer to your home directory.

This depends on the root key.

In shared repositories you usually want to set that key in the repository's root directory, otherwise you will not get stable formatting between the team members thereby defeating the purpose of .editorconfig in the first place.


Also note that editorconfig entries are a list of patterns to match filenames. And we also support matching by language (#664 (comment)). Fitting all this into a flag seems odd.

This issue asks for a file-based exclusion not a language-based exclusion.

See the common case above why people might want to exclude files.

If one has complex needs it is better to use a config file (.editorconfig) no matter what tool one uses.

@sdavids
Copy link
Contributor Author

sdavids commented Sep 20, 2024

Also, as far as I can tell, all options (but the ignore option) supported by .editorconfig are available as CLI flags.

@sdavids sdavids changed the title Add --exclude <glob> parameter Add --ignore <glob> parameter Sep 20, 2024
@sdavids
Copy link
Contributor Author

sdavids commented Sep 20, 2024

Another reason I found:

https://github.com/mvdan/sh/blob/master/cmd/shfmt/shfmt.1.scd

If any parser or printer flags are given to the tool, no EditorConfig formatting options will be used. A default like -i=0 can be used for this purpose.

Well, it cannot be used for that purpose because ignore will be ignored in that case (pun intended).

@mvdan
Copy link
Owner

mvdan commented Sep 29, 2024

In shared repositories you usually want to set that key in the repository's root directory, otherwise you will not get stable formatting between the team members thereby defeating the purpose of .editorconfig in the first place.

I only suggest a parent .editorconfig file for cases where the project does not want such a file committed, like in your original example. Yes, if the project does have such a file committed, then add the entry with ignore. If you think that's too generic, then let's track that in #1094.

EditorConfig files also stack. You can drop an .editorconfig file in the subdirectory to be ignored if you wish. It's really pretty flexible for this use case.

This issue asks for a file-based exclusion not a language-based exclusion.

I would rather not offer, maintain, and document two parallel and different ways to ignore files and directories.

Well, it cannot be used for that purpose because ignore will be ignored in that case (pun intended).

It won't. I think the man page is already clear about this, because ignore=true is not a formatting option (it does not affect the parsing or printing of shell), but I've added a test now to lock in this behavior.

I'm going to close this for now. I appreciate your passionate arguments, but there's a high bar to adding more options and features to this tool.

mvdan added a commit that referenced this issue Sep 29, 2024
The man page says:

    If any parser or printer flags are given to the tool, no
    EditorConfig formatting options will be used. A default like -i=0
    can be used for this purpose.

Since ignore=true is not a formatting option, and it's still useful
when formatting options are given as flags, it should still work then.
The docs said so, but we had no specific test for it.

For #1095.
@mvdan mvdan closed this as not planned Won't fix, can't repro, duplicate, stale Sep 29, 2024
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

No branches or pull requests

2 participants