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

random fixes and maybe improvements #147

Merged
merged 28 commits into from
Oct 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
docker/user_data
.mypy_cache
**/__pycache__
venv
.git
**/.idea

2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,5 @@ tmp/
extension/dist

geckodriver.log

.idea
1 change: 1 addition & 0 deletions README.org
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ Also check out my [[https://beepb00p.xyz/myinfra.html#promnesia][infrastructure
- backend

- simplest: install from [[https://pypi.org/project/promnesia][PyPi]]: =pip3 install --user promnesia=
- install optional dependencies with: =pip3 install --user bs4 lxml mistletoe logzero=
- alternatively: you can clone this repository and run it as ~scripts/promnesia~

This is mainly useful for tinkering with the code and writing new modules.
Expand Down
1 change: 1 addition & 0 deletions doc/DEVELOPMENT.org
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ I'm in the process of putting up better developer's documentation, but you shoul
: --publish # [optional], release to the Chrome Web Store/Mozilla addons

You'll find the result in =dist/=. After that, you can load it in your browser and develop.
For firefox, note that mozilla intentionally complicates this in several ways. On android, only firefox nightly will let you install arbitrary extensions. On desktop, you have a choice between running nightly, re-installing your unpacked extension after every firefox restart, or setting up signing through web-ext. For a quick start into the extension development, on desktop, chrome is therefore recommended (and is the author's primary browser, i believe?). On mobile, firefox nightly is your only option (?).

* Run end-to-end tests

Expand Down
25 changes: 0 additions & 25 deletions docker-compose.yaml

This file was deleted.

1 change: 1 addition & 0 deletions docker/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
user_data/
10 changes: 7 additions & 3 deletions docker/Dockerfile → docker/docker_files/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
FROM python:3

RUN mkdir /data \
RUN mkdir /user_data \
mkdir /usr/src/promnisia

WORKDIR /usr/src/promnesia
COPY src/ .
COPY setup.py /usr/src/

#RUN python /usr/src/setup.py #LookupError: setuptools-scm was unable to detect version for '/usr/src/promnesia'.

RUN pip install --no-cache-dir more_itertools pytz sqlalchemy cachew \
appdirs urlextract python-magic \
tzlocal hug \
logzero HPI beautifulsoup4 lxml mistletoe orgparse dataset

ENV PPATH=/usr/src/promnesia:${PPATH}
VOLUME /data
VOLUME /user_data

EXPOSE 13131
CMD ["python", "-m", "promnesia", "serve", "--db", "/data/promnesia.sqlite", "--port", "13131"]
CMD ["python", "-m", "promnesia", "serve", "--db", "/user_data/promnesia.sqlite", "--port", "13131"]
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
FROM promnesia:latest

RUN apt-get update && apt-get install -y cron
COPY docker/indexer-entrypoint.sh /
COPY docker/docker_files/indexer-entrypoint.sh /
ENTRYPOINT ["/indexer-entrypoint.sh"]
27 changes: 27 additions & 0 deletions docker/docker_files/docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
version: '3.3'

services:
server:
image: promnesia
build:
context: ../../
dockerfile: docker/docker_files/Dockerfile
# Uncomment to enable persisent volumes
volumes:
- ../user_data:/user_data
ports:
- "13131:13131"
restart: always
indexer:
depends_on:
- server
image: promnesia-indexer
build:
context: ../../
dockerfile: docker/docker_files/Dockerfile-indexer
# Uncomment to enable persisent volumes
volumes:
- ../user_data:/user_data
environment:
# run once every ten minutes
CRONTAB: "0-59/1 * * * * cd /usr/src/promnesia && /usr/local/bin/python -m promnesia index --config /user_data/indexer-config.py"
21 changes: 21 additions & 0 deletions docker/docker_files/indexer-config.py.example
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from promnesia import Source
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, not sure about moving this file.. some tests depend on it (and also linked in readme).
But on the other hand it's nice to have some default docker config too.. maybe just create a separate one (could also have less general comments, just essential stuff for running this specific docker setup)?

from promnesia.sources import auto

# todo: we should probably have separate docker volumes for sources and for config/db

SOURCES = [
Source(
auto.index,
'/user_data/source1/',
),
# Source(
# auto.index,
# '/source2/',
# )
]

OUTPUT_DIR = '/user_data/'

# this is not supported yet. also, it should probably be named something else than MIME_HANDLER.
#import os
#MIME_HANDLER = 'editor://' + os.path.realpath(os.path.dirname(os.path.realpath(__file__)) + '../')
7 changes: 7 additions & 0 deletions docker/docker_files/indexer-entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash

# note: https://lostindetails.com/articles/How-to-run-cron-inside-Docker
# note: CRONTAB is set in docker-compose.yaml.

echo "${CRONTAB} > /proc/1/fd/1 2>/proc/1/fd/2" | crontab -
cron -f
11 changes: 11 additions & 0 deletions docker/get-some-data.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/usr/bin/env bash

cd "$(dirname "$0")"

cd user_data/
mkdir source1
cd source1
echo "i like https://github.com/karlicoss/promnesia." >> my_notes.txt
git clone https://github.com/karlicoss/exobrain
git clone https://github.com/koo5/notes

4 changes: 0 additions & 4 deletions docker/indexer-entrypoint.sh

This file was deleted.

9 changes: 9 additions & 0 deletions docker/init.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/env bash

cd "$(dirname "$0")"
mkdir user_data
cp docker_files/indexer-config.py.example user_data/indexer-config.py
./get-some-data.sh

# the config file will be periodically reloaded by the indexer process, and data sources will be periodically re-indexed.

6 changes: 6 additions & 0 deletions docker/start.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/usr/bin/env bash

cd "$(dirname "$0")"
docker-compose -f docker_files/docker-compose.yaml build && docker-compose -f docker_files/docker-compose.yaml up


4 changes: 2 additions & 2 deletions extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
"style-loader": "^1.1.3",
"web-ext": "^4.1.0",
"web-ext-submit": "^4.1.0",
"webpack": "^4.41.6",
"webpack-cli": "^3.3.11",
"webpack": "^4.44.2",
"webpack-cli": "^3.3.12",
"webpack-dev-server": "^3.10.3",
"webpack-extension-manifest-plugin": "^0.5.0"
},
Expand Down
29 changes: 18 additions & 11 deletions extension/src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ async function queryBackendCommon<R>(params, endp: string): Promise<R> {
// right, fetch API doesn't reject on HTTP error status...
const ok = response.ok;
if (!ok) {
throw response.statusText; // TODO...
throw Error(response.statusText + ' (' + response.status + ')'); // TODO...
}
return response.json();
});
Expand All @@ -117,6 +117,8 @@ function getDelayMs(/*url*/) {
return 10 * 1000;
}

//hmm having a space in a tag shouldn't cause an error but it does
//
const LOCAL_TAG = 'local';


Expand Down Expand Up @@ -166,7 +168,10 @@ export async function getVisits(url: Url): Promise<Result> {
// it's gona be a mess though..
const backendRes: Visits | Error = await getBackendVisits(url)
.catch((err: Error) => err);
//console.log(backendRes);

if (backendRes instanceof Error) {
console.log('backend server request error:', backendRes);
return backendRes;
}

Expand Down Expand Up @@ -200,7 +205,7 @@ function getIconStyle(visits: Result): IconStyle {

const vcount = visits.visits.length;
if (vcount === 0) {
return {icon: 'images/ic_not_visited_48.png', title: 'Not visited', text: ''};
return {icon: 'images/ic_not_visited_48.png', title: 'No data', text: ''};
}
const cp = [];

Expand Down Expand Up @@ -229,7 +234,7 @@ function getIconStyle(visits: Result): IconStyle {
const boring = visits.visits.every(v => v.tags.length == 1 && v.tags[0] == LOCAL_TAG);
if (boring) {
// TODO not sure if really worth distinguishing..
return {icon: "images/ic_boring_48.png" , title: `${vcount} visits (local only)`, text: ''};
return {icon: "images/ic_boring_48.png" , title: `${vcount} visits (${LOCAL_TAG} only)`, text: ''};
} else {
return {icon: "images/ic_blue_48.png" , title: `${vcount} visits`, text: ''};
}
Expand All @@ -255,11 +260,11 @@ async function updateState (tab: chrome$Tab) {
.then(() => chromeTabsInsertCSS(tabId, {code: opts.position_css}));


// NOTE: if the page is unreachable, we can't inject stuf in it
// NOTE: if the page is unreachable, we can't inject stuff in it
// not sure how to detect it? tab doesn't have any interesting attributes
// firefox sets tab.title to "Server Not Found"? (TODO also see isOk logic below)
// TODO in this case, could set browser action to open a new tab (i.e. search) or something?
await defensify(inject, 'sidebar injection')();
await defensify(inject, 'sidebar injection for tabId: ${tabId} url: ${url}')();
// TODO crap, at first I forgot () at the end, and flow didn't complain which resulted in flakiness wtf??

const visits = await getVisits(url);
Expand Down Expand Up @@ -458,7 +463,7 @@ function isSpecialProtocol(url: string): boolean {

function ignored(url: string): boolean {
if ([
'https://www.google.com/_/chrome/newtab?ie=UTF-8', // ugh, not sure how to dix that properly
'https://www.google.com/_/chrome/newtab?ie=UTF-8', // ugh, not sure how to fix that properly
'about:blank', // not sure why about:blank is loading like 5 times.. but this seems to fix it
].includes(url)) {
return true;
Expand Down Expand Up @@ -665,7 +670,7 @@ export async function injectSidebar(tab: chrome$Tab) {
const tid = unwrap(tab.id);
if (ignored(url)) {
// TODO tab notification?
notify(`${url} can't be handled`);
notify(`${url} is an ignored URL`);
return;
}
const blacklist = await Blacklist_get();
Expand Down Expand Up @@ -693,7 +698,7 @@ const onCommandCallback = defensify(async cmd => {
} else if (cmd === COMMAND_SEARCH) {
await handleOpenSearch();
} else {
// TODO throw?
// TODO throw? // yea probably
lerror("unexpected command %s", cmd);
}
}, 'onCommand');
Expand Down Expand Up @@ -782,7 +787,7 @@ function initBackground() {
if (android) {
return;
}

// $FlowFixMe // err, complains at Promise but nevertheless works
chrome.commands.onCommand.addListener(onCommandCallback);

Expand All @@ -802,7 +807,7 @@ function initBackground() {
'contexts' : ['page', 'browser_action'],
'title' : "Search in browsing history",
})

// $FlowFixMe // err, complains at Promise but nevertheless works
chrome.contextMenus.onClicked.addListener(onMenuClickedCallback);
})
Expand All @@ -822,7 +827,6 @@ chrome.runtime.onMessage.addListener((info: any, sender: chrome$MessageSender) =
console.log("onmessage %o %o", info, sender);
const aurl = sender.tab == null ? null : sender.tab.url;

console.info("Registering background page callbacks %s", aurl);
if (backgroundInitialised) {
console.debug("background already initialised");
return;
Expand All @@ -833,6 +837,9 @@ chrome.runtime.onMessage.addListener((info: any, sender: chrome$MessageSender) =
lwarn("Suppressing special background page %s", aurl);
return;
}

console.info("Registering background page callbacks in tab %s", aurl);
karlicoss marked this conversation as resolved.
Show resolved Hide resolved

/* TODO not sure if ok or not to await? it shouldn't be blocking right? */
initBackground();
});
10 changes: 10 additions & 0 deletions extension/src/display.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,16 @@ export class Binder {
link.title = 'Jump to the context';
// $FlowFixMe
link.href = loc.href;

// _self seems to "work" only for the "editor://" protocol. Avoids opening a new tab for "editor://" links. Nttp links then require a middle-click, which is undesirable. With normal click, they would not open at all.
// testing on firefox mobile would be useful.
// note that on some pages, like https://news.ycombinator.com/, this (clicking on a _self editor:// link) results in: Content Security Policy: The page’s settings blocked the loading of a resource at editor:///home/koom/promnesia/docker//user_data/source1/notes/g1:12 (“frame-src”).
// but middle-click still works.
Copy link
Owner

Choose a reason for hiding this comment

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

Right... annoying. I guess this is another usecase where some dynamic JS hooks would be useful (so the users can experiment/work around for themselves).

// on others (https://www.reddit.com/), it just works.
// so, if we should do this at all is a question.
//if (link.href.startsWith('editor://'))
// link.target= '_self';

tchild(link, loc.title);
}

Expand Down
15 changes: 8 additions & 7 deletions extension/src/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ class Sidebar {
// makes it much easier for settings
cbody.id = SIDEBAR_ID;
{
const show_dots = cdoc.createElement('button');
show_dots.appendChild(cdoc.createTextNode('Mark visited'));
const show_dots_button = cdoc.createElement('button');
show_dots_button.appendChild(cdoc.createTextNode('Mark visited'));
// TODO hmm. not sure if defensify is gonna work from here? no access to notifications api?
show_dots.addEventListener('click', defensify(async () => {
show_dots_button.addEventListener('click', defensify(async () => {
await chromeRuntimeSendMessage({method: Methods.MARK_VISITED});
}, 'mark_visited.onClick'));
// TODO maybe highlight or just use custom class for that?
show_dots.title = "Mark visited links on the current page with dots";
cbody.appendChild(show_dots);
show_dots_button.title = "Mark visited links on the current page with dots";
cbody.appendChild(show_dots_button);
}
{
const searchb = cdoc.createElement('button');
Expand Down Expand Up @@ -165,7 +165,7 @@ class Sidebar {
this.setupFrame(sidebar);

// TODO a bit nasty, but at the moment easiest way to solve it
// apparetnly iframe is loading about:blank
// apparently iframe is loading about:blank


// TODO perhaps better move it to toggle? although maybe not necessary at all
Expand Down Expand Up @@ -445,7 +445,8 @@ async function bindSidebarData(response: JsonObject) {

// hmm. otherwise it can't be called from executescript??
window.bindSidebarData = bindSidebarData;
window.bindError = bindError;
window.bindError = bindError;


// TODO ugh, it actually seems to erase all the class information :( is it due to message passing??
// eslint-disable-next-line no-unused-vars
Expand Down
2 changes: 1 addition & 1 deletion extension/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const commandsExtra = {
// TODO ugh it's getting messy...
const action = {
"default_icon": "images/ic_not_visited_48.png",
"default_title": "Was not visited",
"default_title": "Show promnesia sidebar",
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe "Not visited (click to show the sidebar)"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good compromise lol! But i wasn't trying to sneak in the "click to show sidebar", rather, i was trying to get rid of the "not visited", which i find misleading:

  1. i will use the sidebar to alert me that maybe my friend wrote some notes about the url. Sure, he surely visited it first, but that's a bit of a stretch. What i want the sidebar to display is more general than "visits", it's any useful information.
  2. if promnesia is having an issue with the database, it doesn't mean the page wasn't visited. Likewise, if i just installed the extension, it will show "not visited", but again that may not be true.

Then again, this was just a bit of bikeshedding to warm me up to the codebase.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah I see!

  1. Yeah, although would need to rethink what information to communicate in the icon anyway
  2. Yep, but ideally you set up the database and it doesn't have issues anymore after that. So worrying about the text when the DB isn't working is the least of your concerns :)

But I guess either way I'm pretty indifferent to the title, because it's only useful the first few times you use the extension, then you just remember which icon is which.

};


Expand Down
Loading