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

fix: Remove extra whitespace in the untitled editor label #108039

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

mixj93
Copy link
Contributor

@mixj93 mixj93 commented Oct 3, 2020

This PR fixes #107979

Normalize multiple whitespaces to a single whitespace for the untitled editor label:

Before:
107979-before

After:
107979-after

@bpasero bpasero self-assigned this Oct 3, 2020
@bpasero bpasero added this to the October 2020 milestone Oct 3, 2020
@mixj93
Copy link
Contributor Author

mixj93 commented Oct 4, 2020

Some Updates:

  • Add logic in case cachedModelFirstLineWords is undefined
  • Change regex /\s{2,}/ to /\s+/ because it's reasonable to replace tab or form feed to one whitespace.

@mixj93 mixj93 closed this Oct 5, 2020
@mixj93 mixj93 reopened this Oct 5, 2020
@mixj93
Copy link
Contributor Author

mixj93 commented Oct 5, 2020

Is there any issue happened in pipline of test? It seems like it cannot process normally. @bpasero

image

@@ -93,7 +93,9 @@ export class UntitledTextEditorModel extends BaseTextEditorModel implements IUnt
// we have no associated file path. In that case we
// prefer the file name as title.
if (this.configuredLabelFormat === 'content' && !this.hasAssociatedFilePath && this.cachedModelFirstLineWords) {
return this.cachedModelFirstLineWords;
// Normalize multiple whitespaces to a single whitespace for the untitled editor label
const normalizedFirstLineWords = this.cachedModelFirstLineWords ? this.cachedModelFirstLineWords.replace(/\s+/g, ' ') : '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not done at the point where we cache the name?

this._onDidChangeName.fire();
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: Normalized when cached the first line words.

@bpasero bpasero marked this pull request as draft October 5, 2020 10:32
@bpasero bpasero modified the milestones: October 2020, On Deck Oct 5, 2020
@mixj93 mixj93 marked this pull request as ready for review October 5, 2020 11:40
@@ -365,7 +365,8 @@ export class UntitledTextEditorModel extends BaseTextEditorModel implements IUnt
}

if (modelFirstWordsCandidate !== this.cachedModelFirstLineWords) {
this.cachedModelFirstLineWords = modelFirstWordsCandidate;
// Normalize multiple whitespaces to a single whitespace for the untitled editor label
this.cachedModelFirstLineWords = modelFirstWordsCandidate ? modelFirstWordsCandidate.replace(/\s+/g, ' ') : '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put this after the .trim() call here:

const firstLineText = this.textEditorModel?.getValueInRange({ startLineNumber: 1, endLineNumber: 1, startColumn: 1, endColumn: UntitledTextEditorModel.FIRST_LINE_NAME_MAX_LENGTH + 1 }).trim();

Because then we do a good check modelFirstWordsCandidate !== this.cachedModelFirstLineWords

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: remove extra spaces after trim first line text.

@bpasero bpasero modified the milestones: On Deck, October 2020 Oct 6, 2020
@bpasero bpasero merged commit a5f5df6 into microsoft:master Oct 6, 2020
@bpasero
Copy link
Member

bpasero commented Oct 6, 2020

Thanks!

@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove extra whitespace from labels for unsaved tabs
2 participants