diff --git a/arduino-ide-extension/src/browser/dialogs/settings/settings-component.tsx b/arduino-ide-extension/src/browser/dialogs/settings/settings-component.tsx index e7fa7a060..9c1563547 100644 --- a/arduino-ide-extension/src/browser/dialogs/settings/settings-component.tsx +++ b/arduino-ide-extension/src/browser/dialogs/settings/settings-component.tsx @@ -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< => { 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 ): 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, + }; + } + } } diff --git a/arduino-ide-extension/src/browser/dialogs/settings/settings-dialog.tsx b/arduino-ide-extension/src/browser/dialogs/settings/settings-dialog.tsx index 67bf4d3d6..9e00eaf78 100644 --- a/arduino-ide-extension/src/browser/dialogs/settings/settings-dialog.tsx +++ b/arduino-ide-extension/src/browser/dialogs/settings/settings-dialog.tsx @@ -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> { 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> { 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 { } 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 { diff --git a/arduino-ide-extension/src/browser/dialogs/settings/settings.tsx b/arduino-ide-extension/src/browser/dialogs/settings/settings.tsx index 44fa7bd0e..d16f49da2 100644 --- a/arduino-ide-extension/src/browser/dialogs/settings/settings.tsx +++ b/arduino-ide-extension/src/browser/dialogs/settings/settings.tsx @@ -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 onDidChange = this.onDidChangeEmitter.event; + protected readonly onDidResetEmitter = new Emitter>(); + readonly onDidReset = this.onDidResetEmitter.event; protected ready = new Deferred(); protected _settings: Settings; @@ -167,7 +169,10 @@ export class SettingsService { async update(settings: Settings, fireDidChange = false): Promise { 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 { 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 { diff --git a/arduino-ide-extension/src/browser/style/settings-dialog.css b/arduino-ide-extension/src/browser/style/settings-dialog.css index 15b5f5bd6..44a641943 100644 --- a/arduino-ide-extension/src/browser/style/settings-dialog.css +++ b/arduino-ide-extension/src/browser/style/settings-dialog.css @@ -57,3 +57,7 @@ display: flex; justify-content: center; } + +.p-Widget.dialogOverlay .dialogBlock .dialogContent.additional-urls-dialog { + display: block; +} diff --git a/arduino-ide-extension/src/common/protocol/config-service.ts b/arduino-ide-extension/src/common/protocol/config-service.ts index aec5f20ea..b1c6285a1 100644 --- a/arduino-ide-extension/src/common/protocol/config-service.ts +++ b/arduino-ide-extension/src/common/protocol/config-service.ts @@ -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; + } +} diff --git a/arduino-ide-extension/src/common/types.ts b/arduino-ide-extension/src/common/types.ts index 85f85f643..421a27534 100644 --- a/arduino-ide-extension/src/common/types.ts +++ b/arduino-ide-extension/src/common/types.ts @@ -1,7 +1,3 @@ export type RecursiveRequired = { [P in keyof T]-?: RecursiveRequired; }; - -export interface Index { - [key: string]: any; -} diff --git a/arduino-ide-extension/src/test/common/config-service.test.ts b/arduino-ide-extension/src/test/common/config-service.test.ts new file mode 100644 index 000000000..aae179770 --- /dev/null +++ b/arduino-ide-extension/src/test/common/config-service.test.ts @@ -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']); + }); + }); +});