When committing unstaged files, can the Git provider remind me about unsaved files?

[bobbrow]

• VSCode Version: Code 1.17.2 (b813d12, 2017-10-16T13:59:46.104Z)
• OS Version: Windows_NT x64 10.0.15063
• Extensions:

Extension	Author (truncated)	Version
EditorConfig	Edi	0.11.1
cpptools	ms-	0.14.0
cmake-tools	vec	0.10.2

---

Reproduces without extensions: Yes

Steps to Reproduce:

1. Open a folder backed by a Git repository
2. Edit some files and save them
3. Edit one of those files again, but don't save it
4. In the Git panel, click the check mark to commit your current changes
5. A message appears asking if you want to automatically stage all your changes and commit them directly
   
6. Click Yes
7. Oops! I still have changes pending because I forgot to save one of my files. 😢

It would be nice if VS Code could detect this and remind me to save my work before staging and committing my changes.

[cleidigh]

@bobbrow
@joaomoreno
I can put this on my November task list.

[itamark]

@bobbrow do you think smartCommit should skip this warning?

[bobbrow]

@itamark are you talking about skipping the warning I'm proposing? I wouldn't want it, but perhaps others would like to disable warnings about unsaved files when commiting all unstaged changes. Perhaps a new setting should control that.

[cleidigh]

@bobbrow
@itamark
@joaomoreno
I will probably post my PR for this tomorrow. I made the following choices for the first pass:

• On any commit a check for unsaved files is performed
• I filtered on:
  • Unsaved/dirty status
  • Schema = file
  • Exclude Untitled documents, dirty or not
  • Exclude unsaved files outside of repository directory tree
  • Check and exclude ignored files
• If the resulting file set > 0, give modal warning with # of unsaved files

If the above does not seem to fit the bill, please comment.
I had thought about the configuration setting as well, I am on the fence on this.
@joaomoreno how do you feel about that?

[joaomoreno]

Sounds great! Yeah a configuration setting would be great too.

[cleidigh]

@joaomoreno
@jrieken
I (had) a working solution to which I added the configuration Boolean. At the same time I updated/pulled from master (and pulled in #37857 ) . This broke the addition and gave the error: Git Invalid token
This occurred when I test with a single file path passed to checkIgnore. I get a return set
with two entries the first is the ignored file (correct) the second is an empty string. The error
is also thrown I presume from the check ignore command call not the function itself which returns
through the resolve. I don't want to post the PR with the debug and problem, but here's the function
to make the checkIgnore call and issue the warning for unsaved files. I also have the patch for the
Promise.resolve bug.

Update: After reading and thinking a bit more is the command processor griping about the null separators from the -z option perhaps? Removing -z appears to correct the error for my case...

private async unsavedResourceCheck(repository: Repository): Promise<string> {

	// Filter all dirty, not those on thisuntitled
	// Check results for ignored files
	// Remove ignored files from list
	// If length > 0, flag unsaved warning

	let dirtyFilePaths = Array.from(workspace.textDocuments.filter(r => {
		return r.isDirty && r.uri.scheme === 'file' && !r.isUntitled &&
		!path.relative(repository.root, r.uri.fsPath).startsWith('..');
	}), r => {
		return r.uri.fsPath;
	});

	this.dumpArray('DirtyFilePaths ', dirtyFilePaths);

	let ignoreSet = new Set<string>();
	try {
		ignoreSet = await repository.checkIgnore(dirtyFilePaths);

	} catch (err) {
		
		console.error(err);
		window.showErrorMessage(err, { modal: false });
		return 'no';
	}

	
	if (ignoreSet.size) {
		ignoreSet.forEach(i => {
			if (i !== undefined && (dirtyFilePaths.indexOf(eval(i)) >= 0)) {
				console.log('RemovingIgnored ' + dirtyFilePaths.indexOf(eval(i)) + ' ' + i);
				dirtyFilePaths.splice(dirtyFilePaths.indexOf(eval(i)), 1);
			}
		});
	}
	console.log('AfterIgnoredCheck');
	this.dumpArray('ReducedDirtyFilePaths ', dirtyFilePaths);

	if (dirtyFilePaths.length) {
		const message = localize('unsavedResourcesMessage', "There {0} ({1}) Unsaved, Tracked {2}.\n\nDo you want to continue with the commit ? ", (dirtyFilePaths.length < 2 ? 'is' : 'are'), dirtyFilePaths.length, (dirtyFilePaths.length < 2 ? 'file' : 'files'));
		const yes = localize('yes', "Yes");
		const no = localize('no', "No");
		const disableWarning = localize('disableWarning', "Disable Warning");
		
		const pick = await window.showWarningMessage(message, { modal: true }, yes, disableWarning);

		if (pick === yes) {
			return 'yes';
		} else if (pick === disableWarning ) {
			return 'disable-warning';
		} else {
			return 'no';
		}
	}
	return 'yes';

}

[cleidigh]

Addressed with PR #38579

Note problem encountered with checkIgnore. May be my problem and how I call the function.

[cleidigh]

fyi @joaomoreno
Found these duplicate related issues and PRs.
slightly different approaches - need to be resolved.

Unsaved files warning before commit

Duplicate Issues:

#37337 - This one
#33004 - Unsaved file warning
#34175 - Slightly different scenario with merge, but essentially same unsaved file warning

Duplicate PRs:

#37681 - for #33004
#36364 - for #33004
#38579 - Mine for #37337

[joaomoreno]

Thanks for the effort @cleidigh... unfortunately I got to #36364 first and liked its approach better for the simplicity. I'm sorry for that.

/duplicate #33004

[vscodebot]

Thanks for creating this issue! We figured it's covering the same as another one we already have. Thus, we closed this one as a duplicate. You can search for existing issues here. See also our issue reporting guidelines.

Happy Coding!