I also have to say use*IgnoreFiles flags are also confusing.

/**
 * Whether external files that exclude files, like .gitignore, should be respected.
 * Defaults to the value for `search.useIgnoreFiles` in settings.
 * For more info, see the setting listed above.
 */
useIgnoreFiles?: boolean;

/**
 * Whether global files that exclude files, like .gitignore, should be respected.
 * Must set `useIgnoreFiles` to `true` to use this.
 * Defaults to the value for `search.useGlobalIgnoreFiles` in settings.
 * For more info, see the setting listed above.
 */
useGlobalIgnoreFiles?: boolean;

/**
 * Whether files in parent directories that exclude files, like .gitignore, should be respected.
 * Must set `useIgnoreFiles` to `true` to use this.
 * Defaults to the value for `search.useParentIgnoreFiles` in settings.
 * For more info, see the setting listed above.
 */
useParentIgnoreFiles?: boolean;

Each flag depends on the associated setting and also on the first flag. I would prefer making it simple and easy to understand

• So setting this flag to false also sets the filesExclude flag to false. Is it easy to understand if these two are made into single property of enum type. Eg: useExcludesConfig: 'files.exclude' | 'search.exclude' | 'none' and defaults to search.exclude ?

This seems to make sense, but I guess there's also the case where the user wants search.exclude to apply if files.exclude is applied in the settings. Not sure whether there's an argument there to make them less paired, or even add an extra case to the enum. Also not sure if that is even a common use case...

but I guess there's also the case where the user wants search.exclude to apply if files.exclude is applied in the settings.

I am under impression that these flags always follow the corresponding settings. And search.exclude setting always include files.include. So not sure if we have such an above case

Right, that is true. So then an enum should be sufficient. For the ignore file case, do you also see that as an enum? Since there are different options (global/parent ignore files), I was thinking of using an object useIgnoreFiles: {parent: boolean, global: boolean} where setting useIgnoreFiles: {} or useIgnoreFiles: true would just enable the local useIgnoreFiles that is required to turn on the parent/global versions?

How does the settings work in this case? Is useParentIgnoreFiles implicitly enables useIgnoreFiles ?

useParentIgnoreFiles does not work without enabling useIgnoreFiles

I suggest a single enum property for ignored files config too. If this property is undefined, defaults to the settings values.

But using a global ignore file doesn't require you to use parent ignore files, so we could have:
local + parent
local + global
local + parent + global

as options. Does this seem like too much to turn into an enum?

I see following options

• none
• local
• local+parent
• local+ global
• all

Right, that makes sense. Is that too many for an enum, especially considering that some of them are combinations?

Yah, I feel this is easy to understand though

Right, that makes sense. I can also see how that is easier to understand. I can gather more feedback about this from others, too.

Fixing this up in #211494. In FindFiles2, there is now excludeSettings and ignoreFilesToUse, which have enums that explicitly say what you are opting into with exclude-setting-following and ignore-file-following.

For a glimpse, here is how the settings have changed.

Option(s) for Following Exclude Setting

Before

	{
		/**
		 * Whether to use the values for files.exclude. Defaults to true.
		 */
		useDefaultExcludes?: boolean;

		/**
		 * Whether to use the values for search.exclude. Defaults to true. Will not be followed if `useDefaultExcludes` is set to `false`.
		 */
		useDefaultSearchExcludes?: boolean;
	}

After:

	{
		/**
		 * Which settings to follow when searching for files. Defaults to {@link ExcludeSettingOptions.searchAndFilesExclude}.
		 */
		excludeSettings?: ExcludeSettingOptions;
	{

with:

	/*
	 * Options for following search.exclude and files.exclude settings.
	 */
	export enum ExcludeSettingOptions {
		/*
		 * Don't use any exclude settings.
		 */
		none = 1,
		/*
		 * Use:
		 * - files.exclude setting
		 */
		filesExclude = 2,
		/*
		 * Use:
		 * - files.exclude setting
		 * - search.exclude setting
		 */
		searchAndFilesExclude = 3
	}

Option(s) for Following Ignore Files

Before

	{
		/**
		 * Whether external files that exclude files, like .gitignore, should be respected.
		 * Defaults to the value for `search.useIgnoreFiles` in settings.
		 * For more info, see the setting listed above.
		 */
		useIgnoreFiles?: boolean;

		/**
		 * Whether global files that exclude files, like .gitignore, should be respected.
		 * Must set `useIgnoreFiles` to `true` to use this.
		 * Defaults to the value for `search.useGlobalIgnoreFiles` in settings.
		 * For more info, see the setting listed above.
		 */
		useGlobalIgnoreFiles?: boolean;

		/**
		 * Whether files in parent directories that exclude files, like .gitignore, should be respected.
		 * Must set `useIgnoreFiles` to `true` to use this.
		 * Defaults to the value for `search.useParentIgnoreFiles` in settings.
		 * For more info, see the setting listed above.
		 */
		useParentIgnoreFiles?: boolean;
	}

After

	{
		/**
		 * Which file locations we should look for ignore (.gitignore or .ignore) files to follow.
		 * Defaults to {@link SearchIgnoreOptions.followSettings}.
		 */
		ignoreFilesToUse?: SearchIgnoreOptions;
	}

with:

	/*
	 * Which locations of .gitignore and .ignore files to follow when searching.
	*/
	export enum SearchIgnoreOptions {
		/*
		 * Don't use ignore files.
		 */
		none = 1,
		/*
		 * Use:
		 * - ignore files at the workspace root.
		 */
		localOnly = 2,
		/*
		 * Use:
		 * - ignore files at the workspace root.
		 *
		 * Follow `search.useParentIgnoreFiles`and `search.useGlobalIgnoreFiles` in settings
		 * to dictate whether to use parent and global ignore files.
		 */
		localAndFollowSettings = 3,
		/*
		 * Use:
		 * - ignore files at the workspace root.
		 * - ignore files directly under parent folder(s) of the workspace root.
		 */
		localAndParent = 4,
		/*
		 * Use:
		 * - ignore files at the workspace root.
		 * - global ignore files (e.g. $HOME/.config/git/ignore).
		 */
		localAndGlobal = 5,
		/*
		 * Use:
		 * - ignore files at the workspace root.
		 * - ignore files directly under parent folder(s) of the workspace root.
		 * - global ignore files (e.g. $HOME/.config/git/ignore).
		 */
		all = 6,
		/*
		* Follow `search.useIgnoreFiles`, `search.useParentIgnoreFiles`, and `search.useGlobalIgnoreFiles` in settings.
		*/
		followSettings = 7,
	}

Some feedback from team members:

• Too many combinations in SearchIgnoreOptions. There should be a more straightforward way by using properties.
• The naming is misleading (ie: useIgnoreFiles is for using local ignore files) - perhaps this is more of a naming issue.
• Perhaps consider not using useIgnoreFiles as a "global" on/off for ignore file support, but rather assume its value if it's not defined and something like useGlobalIgnoreFiles is turned on. Otherwise, if it's an invalid combination, send a warning log.
• It's confusing that the useIgnoreFiles-adjacent properties follow settings when undefined. We should adopt a type using string consts like (auto|forceTrue|forceFalse), which we use in

  vscode/extensions/markdown-language-features/server/src/configuration.ts

  Line 17 in da15b03

  readonly preferredMdPathExtensionStyle: 'auto' | 'includeExtension' | 'removeExtension';

• We could also leverage objects to make this clearer.

Brainstorming

From the feedback, we could do something like:

Option 1

export type IgnoreFileEnabled= 'auto' | 'forceTrue' | 'forceFalse' ;

{
     useIgnoreFiles: {
          local?: IgnoreFileEnabled
          parent?: IgnoreFileEnabled
          global?: IgnoreFileEnabled
     }
}

And just error log if someone does something like

useIgnoreFiles: {
    local: 'forceFalse',
    parent: 'forceTrue'
}

Because we assume local to be 'forceTrue' if parent is 'forceTrue'.

Option 2

We could also go about this like

{
    useLocalIgnoreFiles?: IgnoreFileEnabled | { withParent: IgnoreFileEnabled, withGlobal:IgnoreFileEnabled}
}

Where

useLocalIgnoreFiles: {
withParent: 'forceTrue',
withGlobal: 'auto'
}

would enforce local gitignore to be 'forceTrue'.

So therefore, using the object to assign in useLocalIgnoreFiles would make local gitignore follow 'forceTrue' behind the scenes and the default being:

useLocalIgnoreFiles: {
withParent: 'auto',
withGlobal: 'auto'
}

if you don't assign useLocalIgnoreFiles at all.

Meanwhile, if you do something like

useLocalIgnoreFiles:'forceTrue'

or

useLocalIgnoreFiles:'auto'

Then we assume forceFalse for using parent/globals.

I like option 2, I think it models the actual domain the best. However, we have this dependency on local ignore files, "Must set useIgnoreFiles to true to use this." but do we actually need that? I can't remember what drives that, is it just a limitation of rg options that I can't enable the global ignore file but disable the workspace one?

We currently leverage the --no-ignore flag when the user doesn't want to use local ignore files. It seems that there's currently no way of only disabling local while enabling other ignore files afaik

After thinking about this a bit, I think that we can go with a version of option 2 with enums instead of string const types (to make it simpler).

This could be what we do for the confusing exclude/ignores that we mention above:

export interface FindFiles2Options {
	...
	/**
	 * Which settings to follow when searching for files. Defaults to {@link ExcludeSettingOptions.searchAndFilesExclude}.
	 */
	excludeSettings?: ExcludeSettingOptions;
	/**
	 * Which file locations we should look for ignore (.gitignore or .ignore) files to follow.
	 * Defaults to { withParent:{@link IgnoreFileEnabled.auto}, withGlobal:{@link IgnoreFileEnabled.auto}}
	 * `withParent` and `withGlobal` can be used if you want to use global or parent folder ignore files.
	 * Whenever parent or global ignore files are followed, local ignore files must be followed.
	 */
	useLocalIgnoreFiles?: IgnoreFileEnabled | { withParent: IgnoreFileEnabled, withGlobal: IgnoreFileEnabled };
	...
}

With the following enums.

// whether or not to force usage of ignore files. `auto` will follow the value for the corresponding `search.use*IgnoreFiles` settting.
export enum IgnoreFileEnabled {
	auto = 1,
	forceTrue = 2,
	forceFalse = 3,
}
/*
	* Options for following search.exclude and files.exclude settings.
	*/
export enum ExcludeSettingOptions {
	/*
	 * Don't use any exclude settings.
	 */
	none = 1,
	/*
	 * Use:
	 * - files.exclude setting
	 */
	filesExclude = 2,
	/*
	 * Use:
	 * - files.exclude setting
	 * - search.exclude setting
	 */
	searchAndFilesExclude = 3
}

@mjbvz This is an improved version from what I presented in the API sync. Does this seem to be clearer?

I like this!

Feedback from API sync:

• we shouldn't limit the ignore file settings based on what ripgrep limits (in case we want to support another engine in the future). People who use the ignore file part of the API should probably be pretty technical anyways, so we should just put error logging if it's an invalid setting and perhaps assume what setting they want.
• there was also some discussion of potentially going back to following exclude settings (files.exclude and search.exclude) as separate flags if they aren't dependent of one another. After another look, it looks like search.exclude naturally inherits all of the values of files.exclude and there isn't any way of only using what is listed in search.exclude. This being said, perhaps the enum is still the best way of going about this. (fyi @jrieken )

This being said, this is a revised version of these options.

export interface FindFiles2Options {
	...
	/**
	 * Which settings to follow when searching for files. Defaults to {@link ExcludeSettingOptions.searchAndFilesExclude}.
	 */
	excludeSettings?: ExcludeSettingOptions;
	/**
	 * Which file locations we should look for ignore (.gitignore or .ignore) files to follow.
	 * When `undefined`, we will follow settings (or assume the value if only one is valid) using the value for the corresponding `search.use*IgnoreFiles` settting.
	 * Any time that `parent` or `global` is set to `true`, `local` must also be `true`.
	 * Will log an error if an invalid combination is set.
	 */
	useIgnoreFiles: {
		local?: boolean;
		parent?: boolean;
		global?: boolean;
	}
	...
}

/*
	* Options for following search.exclude and files.exclude settings.
	*/
export enum ExcludeSettingOptions {
	/*
	 * Don't use any exclude settings.
	 */
	none = 1,
	/*
	 * Use:
	 * - files.exclude setting
	 */
	filesExclude = 2,
	/*
	 * Use:
	 * - files.exclude setting
	 * - search.exclude setting
	 */
	searchAndFilesExclude = 3
}

we shouldn't limit the ignore file settings based on what ripgrep limits

I agree with this in principle, but it also seems a little weird to use the ignore file in a parent folder while not using the ignore file in the current folder. You might check whether some other search tool lets you do that kind of thing, or if something like git lets you do that kind of thing.