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

Add Regex/Bulk Edits #3671

Merged
merged 11 commits into from
May 3, 2023
48 changes: 48 additions & 0 deletions src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,18 @@
"message": "$1 edited tracks",
"description": "'Edited tracks' header"
},
"optionsRegexEdits": {
"message": "Regex/Bulk edits",
"description": "'Regex/Bulk edits' header"
},
"optionsRegexEditsDesc": {
"message": "This extension stores regex/bulk edits.",
"description": "Description of regex/bulk edits"
},
"optionsRegexEditsPopupTitle": {
"message": "$1 regex/bulk edits",
"description": "'Regex/Bulk edits' header"
},
"optionsViewEdited": {
"message": "View",
"description": "Button to view edited tracks"
Expand Down Expand Up @@ -495,6 +507,14 @@
"message": "You cannot swap empty track info",
"description": "Title of swap button"
},
"infoRegexTitle": {
"message": "Regex/Bulk edit mode",
"description": "Title of regex/bulk edit button"
},
"infoEditedWarning": {
"message": "This track has been edited. Regex edits are not applied to edited tracks, and the preview you see here may be inaccurate.",
"description": "Warning message to user if bulk editing on a previously edited track"
},
"infoSubmitTitle": {
"message": "Submit changes",
"description": "Title of submit button"
Expand Down Expand Up @@ -543,6 +563,34 @@
"message": "Album artist (optional)",
"description": "Placeholder of album artist input"
},
"infoSearchLabel": {
"message": "Search",
"description": "Label of regex edit search input"
},
"infoReplaceLabel": {
"message": "Replace",
"description": "Label of regex edit replace input"
},
"infoPreviewLabel": {
"message": "Preview",
"description": "Label of regex edit preview output"
},
"infoArtistLabel": {
"message": "Artist",
"description": "Label of artist input row"
},
"infoTrackLabel": {
"message": "Track",
"description": "Label of track input row"
},
"infoAlbumLabel": {
"message": "Album",
"description": "Label of album input row"
},
"infoAlbumArtistLabel": {
"message": "Album artist",
"description": "Label of album artist input row"
},
"infoViewArtistPage": {
"message": "View artist page: $1",
"description": "Placeholder of album input"
Expand Down
3 changes: 3 additions & 0 deletions src/core/background/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
import ClonedSong from '@/core/object/cloned-song';
import { openTab } from '@/util/util-browser';
import { updateAction } from './action';
import { setRegexDefaults } from '@/util/regex';

const disabledTabs = BrowserStorage.getStorage(BrowserStorage.DISABLED_TABS);

Expand Down Expand Up @@ -293,6 +294,8 @@ function startupFunc() {
state.set(DEFAULT_STATE);
disabledTabs.set({});

setRegexDefaults();

browser.contextMenus.create({
id: contextMenus.ENABLE_CONNECTOR,
visible: false,
Expand Down
3 changes: 2 additions & 1 deletion src/core/object/pipeline/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ export async function process(
(song.processed[field] as any) = songInfo[field];
}

if (!song.getAlbum()) {
if (!song.getAlbum() || song.flags.isAlbumFetched) {
song.processed.album = songInfo.album;
song.flags.isAlbumFetched = true;
}
}

Expand Down
10 changes: 9 additions & 1 deletion src/core/object/pipeline/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,21 @@
import * as UserInput from '@/core/object/pipeline/user-input';
import * as Metadata from '@/core/object/pipeline/metadata';
import * as Normalize from '@/core/object/pipeline/normalize';
import * as RegexEdits from '@/core/object/pipeline/regex-edits';
import * as CoverArtArchive from '@/core/object/pipeline/coverartarchive/coverartarchive';
import Song from '@/core/object/song';
import { ConnectorMeta } from '@/core/connectors';

export default class Pipeline {
private song: Song | null = null;
private processors = [Normalize, UserInput, Metadata, CoverArtArchive];
private processors = [
Normalize,
UserInput,
RegexEdits,
Metadata,
RegexEdits, // Run regex edits again, as the regex edit might have caused an album to be found.
yayuyokitano marked this conversation as resolved.
Show resolved Hide resolved
CoverArtArchive,
];
constructor() {
this.song = null;
}
Expand Down
17 changes: 17 additions & 0 deletions src/core/object/pipeline/regex-edits.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import Song from '@/core/object/song';
import RegexEdits from '@/core/storage/regex-edits';

/**
* Fill song info by user defined regex edits.
* @param song - Song instance
*/
export async function process(song: Song): Promise<void> {
let isSongRegexLoaded = false;
try {
isSongRegexLoaded = await RegexEdits.loadSongInfo(song);
} catch (e) {
// Do nothing
Copy link
Member

Choose a reason for hiding this comment

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

Should we log debug here to make debugging easier if something really went wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure. To be honest I dont know how we would even get an error here.

But this pipeline is directly copied from pipeline/user-input, as the user-input function works basically the same due to regex edit class being directly based off the user edit model.

I assume the try catch is there for some reason, but there was no comment in the original telling me why, so I mindlessly copy pasted.

We could add some logging for sure, it shouldnt spam the user.

I’ll merge this as is for now and I can make a new PR adding some logging later, as due to the size of the changes on this PR this not being merged is blocking several future modifications needed before release.

}

song.flags.isRegexEditedByUser = isSongRegexLoaded;
}
12 changes: 12 additions & 0 deletions src/core/object/song.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export type Flags =
| {
isScrobbled: boolean;
isCorrectedByUser: boolean;
isRegexEditedByUser: boolean;
isAlbumFetched: boolean;
isValid: boolean;
isMarkedAsPlaying: boolean;
isSkipped: boolean;
Expand Down Expand Up @@ -361,6 +363,16 @@ export default class Song extends BaseSong {
*/
isCorrectedByUser: false,

/**
* Flag indicating song info has been affected by a user regex/bulk edit
*/
isRegexEditedByUser: false,

/**
* Flag indicating that the album of the current track was fetched from the Last.fm API
*/
isAlbumFetched: false,

/**
* Flag indicated song is known by scrobbling service.
*/
Expand Down
29 changes: 29 additions & 0 deletions src/core/storage/browser-storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,34 @@ export const STATE_MANAGEMENT = 'StateManagement';
*/
export const DISABLED_TABS = 'DisabledTabs';

/**
* This storage contains the regex/bulk edits saved by the user.
* The format of storage data is following:
* [
* ...,
* \{
* search: \{
* Track: 'Track search regex or null',
* Artist: 'Artist search regex or null',
* Album: 'Album search regex or null',
* AlbumArtist: 'AlbumArtist search regex or null',
* \},
* replace: \{
* Track: 'Track replace regex or null',
* Artist: 'Artist replace regex or null',
* Album: 'Album replace regex or null',
* AlbumArtist: 'AlbumArtist replace regex or null',
* \},
* \},
* ...
* ]
*
* All non-null search properties must be true for the edit to be applied.
* It will then apply all non-null replace properties where search is non-null.
* The latest regex edit to match will be applied, assuming no local cache edits exist.
*/
export const REGEX_EDITS = 'RegexEdits';

/**
* This storage contains the data saved and used by the extension core.
* The format of storage data is following:
Expand All @@ -100,6 +128,7 @@ const storageTypeMap = {
Maloja: LOCAL,

[LOCAL_CACHE]: LOCAL,
[REGEX_EDITS]: LOCAL,
[CORE]: LOCAL,
[STATE_MANAGEMENT]: LOCAL,
[DISABLED_TABS]: LOCAL,
Expand Down
79 changes: 79 additions & 0 deletions src/core/storage/regex-edits.model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { REGEX_EDITS } from './browser-storage';
import { CustomStorage } from './custom-storage';
import { DataModels } from './wrapper';
import {
RegexEdit,
RegexFields,
editSong,
shouldApplyEdit,
} from '@/util/regex';
import Song from '../object/song';

type K = typeof REGEX_EDITS;
type V = DataModels[K];
type T = Record<K, V>;

export default abstract class RegexEditsModel extends CustomStorage<K> {
public abstract getRegexEditStorage(): Promise<RegexEdit[] | null>;
public abstract saveRegexEditToStorage(data: T[K]): Promise<void>;

/**
* Apply regex edits to a given song object.
*
* @param song - Song instance
* @returns True if data is loaded; false otherwise
*/
async loadSongInfo(song: Song): Promise<boolean> {
const storageData = await this.getRegexEditStorage();

if (!storageData) {
return false;
}

for (let i = storageData.length - 1; i >= 0; i--) {
if (!shouldApplyEdit(storageData[i].search, song)) {
continue;
}
editSong(storageData[i], song);
return true;
}

return false;
}

/**
* Save custom regex edit to the storage.
*
* @param search - Search to save
* @param replace - Replace to save
*/
async saveRegexEdit(
search: RegexFields,
replace: RegexFields
): Promise<void> {
const storageData = await this.getRegexEditStorage();
if (storageData === null) {
await this.saveRegexEditToStorage([{ search, replace }]);
return;
}

const newStorageData = [
...storageData,
{
search,
replace,
},
];

await this.saveRegexEditToStorage(newStorageData);
}

async deleteRegexEdit(index: number): Promise<void> {
const storageData = await this.getRegexEditStorage();
if (storageData === null) {
return;
}

await this.saveRegexEditToStorage(storageData.splice(index, 1));
}
}
38 changes: 38 additions & 0 deletions src/core/storage/regex-edits.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { REGEX_EDITS } from './browser-storage';
import StorageWrapper, { DataModels } from './wrapper';
import * as BrowserStorage from './browser-storage';
import { RegexEdit } from '@/util/regex';
import RegexEditsModel from './regex-edits.model';

type K = typeof REGEX_EDITS;
type V = DataModels[K];
type T = Record<K, V>;

class RegexEditsImpl extends RegexEditsModel {
private regexEditStorage = this.getStorage();

constructor() {
super(BrowserStorage.getLocalStorage(BrowserStorage.REGEX_EDITS));

/* @ifdef DEBUG */
void this.regexEditStorage.debugLog();
/* @endif */
}

/** @override */
async getRegexEditStorage(): Promise<RegexEdit[] | null> {
return this.regexEditStorage.get();
}

/** @override */
async saveRegexEditToStorage(data: T[K]): Promise<void> {
return await this.regexEditStorage.set(data);
}

/** @override */
getStorage(): StorageWrapper<K> {
return BrowserStorage.getStorage(BrowserStorage.REGEX_EDITS);
}
}

export default new RegexEditsImpl();
3 changes: 3 additions & 0 deletions src/core/storage/wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
LOCAL_CACHE,
NOTIFICATIONS,
OPTIONS,
REGEX_EDITS,
STATE_MANAGEMENT,
StorageNamespace,
} from '@/core/storage/browser-storage';
Expand All @@ -22,6 +23,7 @@ import { ControllerModeStr } from '@/core/object/controller/controller';
import { CloneableSong } from '@/core/object/song';
import EventEmitter from '@/util/emitter';
import connectors from '@/core/connectors';
import { RegexEdit } from '@/util/regex';

export interface CustomPatterns {
[connectorId: string]: string[];
Expand Down Expand Up @@ -76,6 +78,7 @@ export interface DataModels extends ScrobblerModels {
/* local options */
[CORE]: { appVersion: string };
[LOCAL_CACHE]: { [key: string]: SavedEdit };
[REGEX_EDITS]: RegexEdit[];

/* state management */
[STATE_MANAGEMENT]: StateManagement;
Expand Down