#919, #881: Fixed 3rd party URLs-related issues (#920)

* Fixed empty string to URLs conversion

Closes #919.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

* #881: Fixed height of the 3rd part URLs `textarea`

Closes #881.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>
This commit is contained in:
Akos Kitta 2022-04-04 15:52:55 +01:00 committed by GitHub
parent c9b498fb08
commit 0db119d7ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 159 additions and 31 deletions

View File

@ -9,6 +9,7 @@ import { WindowService } from '@theia/core/lib/browser/window/window-service';
import { FileDialogService } from '@theia/filesystem/lib/browser/file-dialog/file-dialog-service';
import { DisposableCollection } from '@theia/core/lib/common/disposable';
import {
AdditionalUrls,
CompilerWarningLiterals,
Network,
ProxySettings,
@ -35,21 +36,32 @@ export class SettingsComponent extends React.Component<
if (
this.state &&
prevState &&
JSON.stringify(this.state) !== JSON.stringify(prevState)
JSON.stringify(SettingsComponent.State.toSettings(this.state)) !==
JSON.stringify(SettingsComponent.State.toSettings(prevState))
) {
this.props.settingsService.update(this.state, true);
this.props.settingsService.update(
SettingsComponent.State.toSettings(this.state),
true
);
}
}
componentDidMount(): void {
this.props.settingsService
.settings()
.then((settings) => this.setState(settings));
this.toDispose.push(
.then((settings) =>
this.setState(SettingsComponent.State.fromSettings(settings))
);
this.toDispose.pushAll([
this.props.settingsService.onDidChange((settings) =>
this.setState(settings)
)
);
this.setState((prevState) => ({
...SettingsComponent.State.merge(prevState, settings),
}))
),
this.props.settingsService.onDidReset((settings) =>
this.setState(SettingsComponent.State.fromSettings(settings))
),
]);
}
componentWillUnmount(): void {
@ -290,8 +302,8 @@ export class SettingsComponent extends React.Component<
<input
className="theia-input stretch with-margin"
type="text"
value={this.state.additionalUrls.join(',')}
onChange={this.additionalUrlsDidChange}
value={this.state.rawAdditionalUrlsValue}
onChange={this.rawAdditionalUrlsValueDidChange}
/>
<i
className="fa fa-window-restore theia-button shrink"
@ -475,11 +487,13 @@ export class SettingsComponent extends React.Component<
protected editAdditionalUrlDidClick = async (): Promise<void> => {
const additionalUrls = await new AdditionalUrlsDialog(
this.state.additionalUrls,
AdditionalUrls.parse(this.state.rawAdditionalUrlsValue, ','),
this.props.windowService
).open();
if (additionalUrls) {
this.setState({ additionalUrls });
this.setState({
rawAdditionalUrlsValue: AdditionalUrls.stringify(additionalUrls),
});
}
};
@ -492,11 +506,11 @@ export class SettingsComponent extends React.Component<
}
};
protected additionalUrlsDidChange = (
protected rawAdditionalUrlsValueDidChange = (
event: React.ChangeEvent<HTMLInputElement>
): void => {
this.setState({
additionalUrls: event.target.value.split(',').map((url) => url.trim()),
rawAdditionalUrlsValue: event.target.value,
});
};
@ -699,5 +713,48 @@ export namespace SettingsComponent {
readonly windowService: WindowService;
readonly localizationProvider: AsyncLocalizationProvider;
}
export type State = Settings & { languages: string[] };
export type State = Settings & {
rawAdditionalUrlsValue: string;
};
export namespace State {
export function fromSettings(settings: Settings): State {
return {
...settings,
rawAdditionalUrlsValue: AdditionalUrls.stringify(
settings.additionalUrls
),
};
}
export function toSettings(state: State): Settings {
const parsedAdditionalUrls = AdditionalUrls.parse(
state.rawAdditionalUrlsValue,
','
);
return {
...state,
additionalUrls: AdditionalUrls.sameAs(
state.additionalUrls,
parsedAdditionalUrls
)
? state.additionalUrls
: parsedAdditionalUrls,
};
}
export function merge(prevState: State, settings: Settings): State {
const prevAdditionalUrls = AdditionalUrls.parse(
prevState.rawAdditionalUrlsValue,
','
);
return {
...settings,
rawAdditionalUrlsValue: prevState.rawAdditionalUrlsValue,
additionalUrls: AdditionalUrls.sameAs(
prevAdditionalUrls,
settings.additionalUrls
)
? prevAdditionalUrls
: settings.additionalUrls,
};
}
}
}

View File

@ -11,6 +11,7 @@ import { FileDialogService } from '@theia/filesystem/lib/browser/file-dialog/fil
import { nls } from '@theia/core/lib/common';
import { SettingsComponent } from './settings-component';
import { AsyncLocalizationProvider } from '@theia/core/lib/common/i18n/localization';
import { AdditionalUrls } from '../../../common/protocol';
@injectable()
export class SettingsWidget extends ReactWidget {
@ -96,7 +97,7 @@ export class SettingsDialog extends AbstractDialog<Promise<Settings>> {
this.update();
}
protected onUpdateRequest(msg: Message) {
protected onUpdateRequest(msg: Message): void {
super.onUpdateRequest(msg);
this.widget.update();
}
@ -105,7 +106,7 @@ export class SettingsDialog extends AbstractDialog<Promise<Settings>> {
super.onActivateRequest(msg);
// calling settingsService.reset() in order to reload the settings from the preferenceService
// and update the UI including changes triggerd from the command palette
// and update the UI including changes triggered from the command palette
this.settingsService.reset();
this.widget.activate();
@ -168,10 +169,7 @@ export class AdditionalUrlsDialog extends AbstractDialog<string[]> {
}
get value(): string[] {
return this.textArea.value
.split('\n')
.map((url) => url.trim())
.filter((url) => !!url);
return AdditionalUrls.parse(this.textArea.value, 'newline');
}
protected onAfterAttach(message: Message): void {

View File

@ -8,8 +8,8 @@ import { ThemeService } from '@theia/core/lib/browser/theming';
import { MaybePromise } from '@theia/core/lib/common/types';
import { FrontendApplicationStateService } from '@theia/core/lib/browser/frontend-application-state';
import { PreferenceService, PreferenceScope } from '@theia/core/lib/browser';
import { Index } from '../../../common/types';
import {
AdditionalUrls,
CompilerWarnings,
ConfigService,
FileSystemExt,
@ -35,7 +35,7 @@ export const UPLOAD_VERBOSE_SETTING = `${UPLOAD_SETTING}.verbose`;
export const UPLOAD_VERIFY_SETTING = `${UPLOAD_SETTING}.verify`;
export const SHOW_ALL_FILES_SETTING = `${SKETCHBOOK_SETTING}.showAllFiles`;
export interface Settings extends Index {
export interface Settings {
editorFontSize: number; // `editor.fontSize`
themeId: string; // `workbench.colorTheme`
autoSave: 'on' | 'off'; // `editor.autoSave`
@ -53,7 +53,7 @@ export interface Settings extends Index {
sketchbookShowAllFiles: boolean; // `arduino.sketchbook.showAllFiles`
sketchbookPath: string; // CLI
additionalUrls: string[]; // CLI
additionalUrls: AdditionalUrls; // CLI
network: Network; // CLI
}
export namespace Settings {
@ -84,6 +84,8 @@ export class SettingsService {
protected readonly onDidChangeEmitter = new Emitter<Readonly<Settings>>();
readonly onDidChange = this.onDidChangeEmitter.event;
protected readonly onDidResetEmitter = new Emitter<Readonly<Settings>>();
readonly onDidReset = this.onDidResetEmitter.event;
protected ready = new Deferred<void>();
protected _settings: Settings;
@ -167,7 +169,10 @@ export class SettingsService {
async update(settings: Settings, fireDidChange = false): Promise<void> {
await this.ready.promise;
for (const key of Object.keys(settings)) {
this._settings[key] = settings[key];
if (key in this._settings) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(this._settings as any)[key] = (settings as any)[key];
}
}
if (fireDidChange) {
this.onDidChangeEmitter.fire(this._settings);
@ -176,7 +181,8 @@ export class SettingsService {
async reset(): Promise<void> {
const settings = await this.loadSettings();
return this.update(settings, true);
await this.update(settings, false);
this.onDidResetEmitter.fire(this._settings);
}
async validate(
@ -267,7 +273,10 @@ export class SettingsService {
// after saving all the settings, if we need to change the language we need to perform a reload
// Only reload if the language differs from the current locale. `nls.locale === undefined` signals english as well
if (currentLanguage !== nls.locale && !(currentLanguage === 'en' && nls.locale === undefined)) {
if (
currentLanguage !== nls.locale &&
!(currentLanguage === 'en' && nls.locale === undefined)
) {
if (currentLanguage === 'en') {
window.localStorage.removeItem(nls.localeId);
} else {

View File

@ -57,3 +57,7 @@
display: flex;
justify-content: center;
}
.p-Widget.dialogOverlay .dialogBlock .dialogContent.additional-urls-dialog {
display: block;
}

View File

@ -116,7 +116,7 @@ export interface Config {
readonly sketchDirUri: string;
readonly dataDirUri: string;
readonly downloadsDirUri: string;
readonly additionalUrls: string[];
readonly additionalUrls: AdditionalUrls;
readonly network: Network;
readonly daemon: Daemon;
}
@ -141,3 +141,32 @@ export namespace Config {
);
}
}
export type AdditionalUrls = string[];
export namespace AdditionalUrls {
export function parse(value: string, delimiter: ',' | 'newline'): string[] {
return value
.trim()
.split(delimiter === ',' ? delimiter : /\r?\n/)
.map((url) => url.trim())
.filter((url) => !!url);
}
export function stringify(additionalUrls: AdditionalUrls): string {
return additionalUrls.join(',');
}
export function sameAs(left: AdditionalUrls, right: AdditionalUrls): boolean {
if (left.length !== right.length) {
return false;
}
const localeCompare = (left: string, right: string) =>
left.localeCompare(right);
const normalize = (url: string) => url.toLowerCase();
const normalizedLeft = left.map(normalize).sort(localeCompare);
const normalizedRight = right.map(normalize).sort(localeCompare);
for (let i = 0; i < normalizedLeft.length; i++) {
if (normalizedLeft[i] !== normalizedRight[i]) {
return false;
}
}
return true;
}
}

View File

@ -1,7 +1,3 @@
export type RecursiveRequired<T> = {
[P in keyof T]-?: RecursiveRequired<T[P]>;
};
export interface Index {
[key: string]: any;
}

View File

@ -0,0 +1,35 @@
import { expect } from 'chai';
import { AdditionalUrls } from '../../common/protocol';
describe('config-service', () => {
describe('additionalUrls', () => {
it('should consider additional URLs same as if they differ in case', () => {
expect(AdditionalUrls.sameAs(['aaaa'], ['AAAA'])).to.be.true;
});
it('should consider additional URLs same as if they have a different order', () => {
expect(AdditionalUrls.sameAs(['bbbb', 'aaaa'], ['aaaa', 'bbbb'])).to.be
.true;
});
it('should parse an empty string as an empty array', () => {
expect(AdditionalUrls.parse('', ',')).to.be.empty;
});
it('should parse a blank string as an empty array', () => {
expect(AdditionalUrls.parse(' ', ',')).to.be.empty;
});
it('should parse urls with commas', () => {
expect(AdditionalUrls.parse(' ,a , b , c, ', ',')).to.be.deep.equal([
'a',
'b',
'c',
]);
});
it("should parse urls with both '\\n' and '\\r\\n' line endings", () => {
expect(
AdditionalUrls.parse(
'a ' + '\r\n' + ' b ' + '\n' + ' c ' + '\r\n' + ' ' + '\n' + '',
'newline'
)
).to.be.deep.equal(['a', 'b', 'c']);
});
});
});