Skip to content

Commit

Permalink
extension: partial fix for whitespace in sources
Browse files Browse the repository at this point in the history
spotted by @koo5 #147
  • Loading branch information
karlicoss committed Nov 4, 2020
1 parent 8d49b82 commit e7781ed
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 5 deletions.
4 changes: 4 additions & 0 deletions doc/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
from promnesia.sources import takeout, instapaper, pocket, fbmessenger, twitter, roamresearch, hypothesis, rss


# NOTE: at the moment try to avoid using complex sources names
# it's best to stick to digits, latin characters, dashes and underscores
# there might be some UI problems related to that

# now we can specify the sources
# this is a required setting
SOURCES = [
Expand Down
1 change: 0 additions & 1 deletion extension/src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ function getDelayMs(/*url*/) {
return 10 * 1000;
}

// TODO: having a space in a tag shouldn't cause an error but it does
// TODO: make it configurable in options?
const LOCAL_TAG = 'local';

Expand Down
14 changes: 12 additions & 2 deletions extension/src/display.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,23 @@ type Params = {

const HTML_MARKER = '!html ';

// todo use opaque type? makes it annoying to convert literal strings...
type CssClass = string
export function asClass(x: string): CssClass {
// todo meh. too much trouble to fix properly...
const res = x.replace(/[^0-9a-z_-]/, '_')
return res.length == 0 ? 'bad_class' : res
}


export class Binder {
doc: Document;

constructor(doc: Document) {
this.doc = doc;
}

makeChild(parent: HTMLElement, name: string, classes: ?Array<string> = null) {
makeChild(parent: HTMLElement, name: string, classes: ?Array<CssClass> = null) {
const res = this.doc.createElement(name);
if (classes != null) {
for (const cls of classes) {
Expand Down Expand Up @@ -86,6 +95,7 @@ export class Binder {
relative,
}: Params,
) {
// todo ugh. looks like Flow can't guess the type of clouser that we get by .bind...
const child = this.makeChild.bind(this);
const tchild = this.makeTchild.bind(this); // TODO still necessary??

Expand Down Expand Up @@ -113,7 +123,7 @@ export class Binder {
tchild(idx_c, String(idx));
}
for (const tag of tags) {
const tag_c = child(tags_c, 'span', ['src', tag]);
const tag_c = child(tags_c, 'span', ['src', asClass(tag)]);
tchild(tag_c, tag);
}
tchild(date_c, dates);
Expand Down
4 changes: 2 additions & 2 deletions extension/src/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {Visits, Visit, unwrap, format_duration, Methods, addStyle} from './commo
import type {JsonObject, Second} from './common';
import {get_options_async, USE_ORIGINAL_TZ, GROUP_CONSECUTIVE_SECONDS} from './options';
import type {Options} from './options';
import {Binder, _fmt} from './display';
import {Binder, _fmt, asClass} from './display';
import {defensify} from './notifications';
import {chromeRuntimeSendMessage} from './async_chrome';

Expand Down Expand Up @@ -353,7 +353,7 @@ async function bindSidebarData(response: JsonObject) {

// TODO show total counts?
// TODO if too many tags, just overlap on the seconds line
const tag_c = binder.makeChild(all_tags_c, 'span', ['src', tag]);
const tag_c = binder.makeChild(all_tags_c, 'span', ['src', asClass(tag)])
binder.makeTchild(tag_c, `${tag} (${count})`);
// TODO checkbox??
tag_c.addEventListener('click', () => {
Expand Down

0 comments on commit e7781ed

Please sign in to comment.