-
-
Notifications
You must be signed in to change notification settings - Fork 776
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
[WIP] Split OMEMO plugin into view and headless components #3117
base: master
Are you sure you want to change the base?
Conversation
This replaces 'plugins/omemo' with 'plugins/omemo-views' and 'headless/plugins/omemo'. Most of the original code is still in the former, but the contents of 'util' has been moved to the headless version. Still pending: moving 'index.js' and all the other files in the plugin!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished moving over utils
, but with a few potential loose ends. @jcbrand I've left four comments for the doubtful areas (tagging you in the first two but then I realised I can avoid spam-tagging by doing it once here 😅).
Let me know if I'm on the right track there? I think I've got the rest under control!
@jcbrand thanks for the comments! Just letting you know I've seen them; will get onto it once I've processed |
The 'omemo-views' plugin is still there, but most of the logic has now been moved to the new headless 'omemo' plugin, with only the UI part remaining in 'omemo-views'.
Turns out there was no headless component to these functions, so nothing to be moved!
@@ -5,7 +5,7 @@ import log from '@converse/headless/log'; | |||
import range from 'lodash-es/range'; | |||
import omit from 'lodash-es/omit'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this file can also be moved to the headless plugin. Generally speaking, all models should go into headless, and OMEMOStore
is a model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, okay! All the tests passed but when I tried running in the browser it's not actually running. So let me debug that and then I'll get to this 🙂
@@ -0,0 +1,137 @@ | |||
/* global libsignal */ | |||
import log from '@converse/headless/log'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class
import { _converse, converse, api } from '@converse/headless/core'; | ||
import { html } from 'lit'; | ||
import { isError } from '@converse/headless/utils/core.js'; | ||
import { isAudioURL, isImageURL, isVideoURL, getURI } from '@converse/headless/utils/url.js'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class
uri, | ||
start, | ||
end, | ||
obj_url: getAndDecryptFile(uri); // this is a promise |
Check notice
Code scanning / CodeQL
Syntax error
@jcbrand I have a problem: the Symptom: I get a Debugging: I tried adding a test console.log inside the initialize() function, and as I suspected it wasn't running. (A similar console.log line inside I tried importing the headless plugin to |
@badrihippo Yes, I think I know what the issue is. You need to add That is necessary in order to register it as a pluggable.js plugin. Otherwise, even though you import it, it's not registered as a plugin and the |
@jcbrand @badrihippo hi, I'm interested in using the converse headless but it doesn't support omemo? any eta on this pr to make it possible? thanks! |
Well, I guess this has become stale 🙁 and also, I've forgotten half of what I did. But let me start afresh and see if I can do a better job this time... |
@badrihippo: Any progress on it? |
@Neustradamus I'm afraid not 🙁 I think I got quite close, but I haven't had time to work on it since that last commit. I think I got pretty close, so if anyone is willing to clone and do the last few fixes to get it running that would help. (@timothyerwin would you be interested?) |
This commit replaces
src/omemo
withplugins/omemo-views
andheadless/plugins/omemo
. The way I'm working on this is to keep everything onomemo-views
for now, and then gradually move whatever possible out to the headlessomemo
.Files to be tackled (more will be added as events warrant):
omemo
plugin separatelyStandard steps to finish before completing pull request:
CHANGES.md
document it in
docs/source/configuration.rst
with
make check
or you can run them in the browser by runningmake serve
and then opening
http://localhost:8000/tests.html
.