improved code based on code review feedback

This commit is contained in:
Till Salinger 2018-02-19 09:13:07 +01:00
parent b7cc9709e8
commit 1d8a10fec2
2 changed files with 29 additions and 46 deletions

View file

@ -1336,14 +1336,11 @@ export function validateFileName(parent: IFileStat, name: string, allowOverwriti
return nls.localize('emptyFileNameError', "A file or folder name must be provided.");
}
const names: string[] = trimTrailingSlashes(name) // prevents empty last array element after split
.split(/[\\/]/);
const names: string[] = name.split(/[\\/]/).filter(part => !!part);
// Do not allow to overwrite existing file
if (!allowOverwriting) {
let p = parent;
const alreadyExisting = names.every((folderName) => {
let { exists, child } = alreadyExists(p, folderName);
@ -1365,8 +1362,10 @@ export function validateFileName(parent: IFileStat, name: string, allowOverwriti
return nls.localize('invalidFileNameError', "The name **{0}** is not valid as a file or folder name. Please choose a different name.", trimLongName(name));
}
// Max length restriction (on Windows)
if (isWindows) {
if (windowsMaxLengthRestrictionReached(name, parent)) {
const fullPathLength = name.length + parent.resource.fsPath.length + 1 /* path segment */;
if (fullPathLength > 255) {
return nls.localize('filePathTooLongError', "The name **{0}** results in a path that is too long. Please choose a shorter name.", trimLongName(name));
}
}
@ -1374,39 +1373,28 @@ export function validateFileName(parent: IFileStat, name: string, allowOverwriti
return null;
}
function windowsMaxLengthRestrictionReached(name: string, parent: IFileStat): boolean {
const fullPathLength = name.length + parent.resource.fsPath.length + 1 /* path segment */;
return fullPathLength > 255;
}
function trimTrailingSlashes(str): string | undefined {
if (!str) { return str; }
return str.replace(/[\\/]*$/, '');
}
function alreadyExists(parent: IFileStat, name: string): { exists: boolean, child: IFileStat | undefined } {
let foundDupChild: IFileStat;
let duplicateChild: IFileStat;
if (parent.children) {
let exists: boolean = parent.children.some((c) => {
let found = compareFolderNames(c.name, name);
if (found) { foundDupChild = c; }
let found: boolean;
if (isLinux) {
found = c.name === name;
} else {
found = c.name.toLowerCase() === name.toLowerCase();
}
if (found) {
duplicateChild = c;
}
return found;
});
return { exists, child: foundDupChild };
return { exists, child: duplicateChild };
}
return { exists: false, child: undefined };
}
function compareFolderNames(name1: string, name2: string): boolean {
if (isLinux) {
return name1 === name2;
}
return name1.toLowerCase() === name2.toLowerCase();
}
function trimLongName(name: string): string {
if (name && name.length > 255) {
return `${name.substr(0, 255)}...`;

View file

@ -296,7 +296,6 @@ export class FileRenderer implements IRenderer {
}, 0);
});
const initialRelPath: string = relative(stat.root.resource.fsPath, stat.parent.resource.fsPath);
const toDispose = [
inputBox,
DOM.addStandardDisposableListener(inputBox.inputElement, DOM.EventType.KEY_DOWN, (e: IKeyboardEvent) => {
@ -309,7 +308,8 @@ export class FileRenderer implements IRenderer {
}
}),
DOM.addStandardDisposableListener(inputBox.inputElement, DOM.EventType.KEY_UP, (e: IKeyboardEvent) => {
displayCurrentPath(inputBox, initialRelPath, fileKind);
const initialRelPath: string = relative(stat.root.resource.fsPath, stat.parent.resource.fsPath);
this.displayCurrentPath(inputBox, initialRelPath, fileKind);
}),
DOM.addDisposableListener(inputBox.inputElement, DOM.EventType.BLUR, () => {
done(inputBox.isInputValid(), true);
@ -318,29 +318,24 @@ export class FileRenderer implements IRenderer {
styler
];
}
}
function displayCurrentPath(inputBox: InputBox, initialRelPath: string, fileKind: FileKind) {
const value = inputBox.value;
if (inputBox.validate()) {
if (value && value.search(/[\\/]/) !== -1) { // only show if there's a slash
const newPath = replaceWithNativeSep(paths.join(initialRelPath, value));
const fileType: string = FileKind[fileKind].toLowerCase();
private displayCurrentPath(inputBox: InputBox, initialRelPath: string, fileKind: FileKind) {
if (inputBox.validate()) {
const value = inputBox.value;
if (value && value.search(/[\\/]/) !== -1) { // only show if there's a slash
const newPath = paths.normalize(paths.join(initialRelPath, value), true);
const fileType: string = FileKind[fileKind].toLowerCase();
inputBox.showMessage({
type: MessageType.INFO,
content: nls.localize('constructedPath', "Create **{0}** in **{1}**", fileType, newPath),
formatContent: true
});
inputBox.showMessage({
type: MessageType.INFO,
content: nls.localize('constructedPath', "Create **{0}** in **{1}**", fileType, newPath),
formatContent: true
});
}
}
}
}
function replaceWithNativeSep(str: string) {
if (!str) { return str; }
return str.replace(/[\\/]/g, paths.nativeSep);
}
// Explorer Accessibility Provider
export class FileAccessibilityProvider implements IAccessibilityProvider {