Closed TheLastZombie closed 2 years ago
If this gets implemented then I think only if the local store toggle enabled for the account. In that mode, we have all the needed data kept in memory which simplifies the implementation.
If this gets implemented then I think only if the local store toggle enabled for the account. In that mode, we have all the needed data kept in memory which simplifies the implementation.
When it’ll be implemented, will it be then possible to view the web UI, enable local store and show the notifications with sender and subject?
Addition feature request point picked from #422:
- If this app will have an option to specify for example a shell script, and execute it with the message from first option as argument, there will appear a possibility to send this arguments where you want, to other email or chat.
When it’ll be implemented, will it be then possible to view the web UI, enable local store and show the notifications with sender and subject?
A user will be able to build a fully custom notification message using any email data, not only "subject" and "sender".
The first part of the feature, showing a custom desktop notification message, is completed in wip-custom-notification-message
branch.
actually show the subject line and either the sender or a preview of that mail (instead of "x unread messages")
A user will be able to build a fully custom notification message using any email data, not only "subject" and "sender".
See the respective demo:
So, as I said before, the feature enables a user to display a fully custom message using any email message data. So code syntax is highlighted with TypeScript support. This means that the email data model is taken from https://github.com/ProtonMail/WebClients/ and injected into the code editor, so you get auto-complete support, code errors highlighting, etc.
By the way, a similar idea with a smart code editor was used in the "JavaScript-based / programmatic messages search" feature which got originally enabled about a year ago in the v4.11.0 release.
At the moment only a plain text message is supported in the notification but technically in the future, it's possible to use a custom native module for showing advanced desktop notification messages (with formatting support, additional custom buttons, etc).
If someone wants to test the feature before it gets released, let me know and I will provide a WIP build.
So the remaining part is the following:
If this app will have an option to specify for example a shell script, and execute it with the message from first option as argument, there will appear a possibility to send this arguments where you want, to other email or chat.
For this need, I think there will be another code editor which will be used for forming a string argument to send to some executable script/binary.
If this app will have an option to specify for example a shell script, and execute it with the message from first option as argument, there will appear a possibility to send this arguments where you want, to other email or chat.
Here it's:
@vladimiry, I tried out Custom notification content
. It is nice, but not perfect.
The default code outputs sender, subject, time of all email messages in inbox from the latest to the oldest. Now, I have 1324 messages in my inbox.
I want to create one notification for every new email I receive. I don’t one notification for all emails.
I can filter the emails to have only Unread
emails, but there is no way to filter out which email was read and then made unread OR which email is new of which I haven’t been notified yet.
TL;DR: Could you add a property which would be true
(1
) only if it haven’t triggered a notification yet? I can request this feature in a separate issue, but I wanted to know if it makes sense to you.
Thanks! :pray:
@tukusejssirs this will be won't fix
as too specific need.
Technically, you can already achieve the need. You would use Shell command execution #387
custom feature/code to process the notification data by a custom script. In this script, you would:
Regarding tracking, "read => unread" status switch. You could use a similar approach of shell custom script to save the "read" flag switching. I'm not putting this logic into the app.
@tukusejssirs this will be
won't fix
as too specific need.
Thanks for reply! :pray: However, I am not quite sure why you think this is a too specific need.
What I want is a a notification that is usual one in any other email client on any platform: get a notification every time a new email arrives with the sender and subject.
Now, when not using the custom desktop notification code, I get a notification every time:
Moreover, you don’t provide the sender and subject by default due to privacy issues which might exist if you are in an untrusted place, however, IMHO most of the users most of the time are checking their emails in a trusted place (like at home). I don’t to argue about this with you, it is you decision and there exists a way to output the subject and sender.
Technically, you can already achieve the need. You would use
Shell command execution #387
custom feature/code to process the notification data by a custom script.
I believe that if ElectronMail saves this as a separate property:
For me (as well as others), it might be better and easier to add a option what should be the notification content:
IMHO if such option (a toggle) would be implemented, this issue would be fully solved and the much more users would be happy using ElectronMail.
Just a note: even ProtonMail client on Android displays the sender and subject.
adding a column and a single command after creating the notification to update the database table row.
Currently, in relation to the local store, there are no such terms as column or table row. Saving local store is currently relatively heavy operation (and decrypting it too), so triggering saving operation more frequently than required by each email status update is not a very good idea. But when/if #571 gets implemented, I might reconsider adding a new flag for email.
Currently, in relation to the local store, there are no such terms as column or table row.
Oh, okay. I was under impression (based on your usage of the term database in the l10n string in the tooltip of the plug icon (Toggle online/database view mode
).
Therefore, yes, it totally makes sense to move to a SQLite before adding new properties/columns. I understand that it might not happen in the near future, but I think it would make the local store much more performant, thus it might be nice to implement it ASAP (I don’t want you hurry nor tell you what should be your priorities). I am not a Go coder, thus I might not be able to help you out in this.
Anyway, I’ll try to use the custom shell command execution to build something when I have some more time for this.
Thanks for you time and ElectronMail! :pray:
Jest a question relating to shell command execution: is the output of formatNotificationShellExecArguments()
function displayed as a notification? Therefore I should use only either custom notification content or shell command execution, right?
is the output of formatNotificationShellExecArguments() function displayed as a notification? Therefore I should use only either custom notification content or shell command execution, right?
Nope. Shell script calling is an independent thing.
Also notice some points from the d9f462dde5c820945f790d5e41beac4636b2b14a commit comment (or release note of https://github.com/vladimiry/ElectronMail/releases/tag/v4.14.0):
- If the resolved "body" is empty then a notification won't be displayed. So this is a way to suppress the notification for a specific account or mail data.
- If the resolved "execution command" is empty then execution won't be happening. So this is a way to suppress the command execution for a specific account or mail data.
Nope. Shell script calling is an independent thing.
Okay, then, I need to (somehow) read/get the data of emails (or at least their IDs) to trigger a notification in the custom notification content function, right? It feels to be cumbersome.
You can see on the screenshot in https://github.com/vladimiry/ElectronMail/issues/387#issuecomment-1087771765 that mails
array is coming into your formatNotificationShellExecArguments
function as an argument. But, I missed pointing out, that currently only unread emails come to that function.
You can see on the screenshot in #387 (comment) that
mails
array is coming into yourformatNotificationShellExecArguments
function as an argument.
I’ve seen that screenshot and I have printed out an item from mails
array, thus I know the structure, but I miss a property that would tell me if the email is new (thus no notification has been triggered yet).
But, I missed pointing out, that currently only unread emails come to that function.
I have no problem with that as new emails are unread emails, but not all unread emails are new emails.
but I miss a property that would tell me if the email is new (thus no notification has been triggered yet).
I've written above that maintaining this property is currently on you (using some external storage/database).
I get that, I didn’t want to suggest anything more.
Another question: is it possible to trigger multiple notifications from formatNotificationContent()
? I want a notification per email, not one notification for all emails. There is a chance that multiple emails would be retrieved at the same time.
I want a notification per email
Currently, the app displays one notification per mail account. But using custom shell script function should alloy you to display as many notifications as needed (if you manage to find a suitable notification showing tool that would support command line interface, a tool could be a self-written too I guess).
@vladimiry, could you tell me why the following code fails running? It runs as expected when I remove the mails
property or when I use mails[0]
.
formatNotificationShellExecArguments(({login, alias, mails}) => {
const data = JSON.stringify({
alias,
date: new Date(),
login,
oneMail: mails[0],
// None of these three values of `mails` work. When I omit then, I get the output, otherwise it raises an error, which is hard to debug, as you use some
mails
// mails: mails.map(i => ({id: i.ID, sender: i.Sender, subject: i.Subject}))
// mails: mails.reduce((a, c) => [...a, {id: c.ID, sender: c.Sender, subject: c.Subject}], [])
// mails: mails.reduce((accumulator, {Subject, Sender, Time}) => [...accumulator, {Sender, Subject, Time}], [])
}).replace(/'/g, "\\'")
return {command: `/usr/bin/echo '${data}' > /home/ts/git/test/desktop_notifs/tmp.json`}
})
// Error when using `mails` or `mails.reduce()` or `mails.map()`:
Uncaught (in promise): Error: Failed to execute a triggered by an unread desktop notification shell exec command!
Error: Failed to execute a triggered by an unread desktop notification shell exec command!
at executeUnreadNotificationShellCommand (/opt/ElectronMail/resources/app.asar/app/electron-main/index.cjs:92077:17)
How could I debug this?
Maybe see if limiting array size helps, using mails.slice(0,10)
instead of mails
. I guess there might be a weak point somewhere that is sensitive to the data portion size. There is no answer at the moment where that weak point is if you confirm data size correlation.
Thanks, @vladimiry, you are right that that is some max data size (maybe a contraint on the size of the used memory, IDK).
According to my tests, I could used data of max 131,011 chars size; anything greater fails. This is my test function that succeeds:
formatNotificationShellExecArguments(({login, alias, mails}) => {
return {command: `/usr/bin/echo '${''.padStart(131011, '1')}' > /some/path/with/42/chars/in/length/including/filename/tmp.json`}
})
// Error
Uncaught (in promise): Error: Failed to execute a triggered by an unread desktop notification shell exec command!
Error: Failed to execute a triggered by an unread desktop notification shell exec command!
at executeUnreadNotificationShellCommand (/opt/ElectronMail/resources/app.asar/app/electron-main/index.cjs:92077:17)
An item from mails
array has approx 15,000 chars in my case, thus 8 items could be used in my case. That said, I don’t need the anything from mails
but ID, sender and subject, thus the size of an item could be reduced to approx 300 chars (min: 187
, max: 376
), thus I can use about 436 items in my case. No chance to read all my emails (1379
) at once, which I would do only to create the initial backup.
@tukusejssirs, thanks for the detailed report. Looks like you hit the issue of maximum character length of arguments in a shell command. I executed the test and got E2BIG error (I've just enabled error code printing, see 1da058c). I think some shell script execution redesigning should be happening in the app, but there is no clear path yet.
No chance to read all my emails (1379) at once, which I would do only to create the initial backup.
Technically, there might be a chance if you put some pure JS string compression implementation into the formatNotificationShellExecArguments
and apply it to the serialized mails
data. You would have to decompress it then in an executed script. But I understand that this sounds too complex thing for a one-off need.
Thanks for the investigation.
Now, I have even thought about writing to file, but node:fs
is not available (as it is an Electron app). Another option would be to create a simple Fastify/Express app with REST API to POST
the data to, process the data there and trigger a notification using node-notifier
there, but I’ve just thought it might be too much. I also wanted the this to be done without any daemon or server running in the background.
For now, I see only one option: iterate over mails
, extract the required properties only, create a temporary array into which we push email data one by one until JSON.stringify(m).length
is less that max allowed char size.
formatNotificationShellExecArguments(({login, alias, mails}) => {
/**
* Max data size in characters in JSON string
*
* @remarks For some reason, `131011` chars does not work this way. Sometimes I get `83781` chars, but not always. Does this character limit counts the size (in chars) of this function?
*/
const maxDataSize = 83499
/**
* Data used by the shell command
*
* @remarks
*
* `data` type is as follows:
*
* type data = {
* a: string, // Account alias
* d: number, // Timestamp when the emails where retrieved
* l: string, // Account login name
* m: ({
* i: string, // Email ID
* sn: string, // Email sender name
* se: string, // Email sender email
* s: string // Email subject
* })[]
* }
*/
const data = {a: alias, d: new Date().getTime(), l: login, m: []}
// Sort `mails` by received data descending
mails.sort((a, b) => b.Time - a.Time)
// Filter `mails` to keep only those properties that are needed by the shell command
for (const i of mails) {
const t = {i: i.ID, sn: i.Sender.Name, se: i.Sender.Email, s: i.Subject}
if (JSON.stringify({...data, m: [...data.m, t]}).replace(/'/g, "\\'").length <= maxDataSize) {
data.m.push(t)
} else {
break
}
}
return {command: `/usr/bin/echo '${JSON.stringify(data).replace(/'/g, "\\'")}' > /home/ts/git/test/desktop_notifs/tmp.json`}
})
I have noticed that mails
in unordered array of emails. At first, I’ve thought that emails I received after 28 Jan 2023 are missing from mails
, but it is not the case. I need to sort the emails. Can’t you sort them by mails[].Time
descending please?
A bit OT, but in the GUI code editor (both for notifications and shell command), it seems (based on the intellisense) like the function is run as module and it does not know about console
and require
(it knows about import
), which is not a problem in itself, but what is that it seems to check the code as if it was a TypeScript code (when I define const data = {a: 1}
, later I add data.b = 2
and the editor raises an error). I have no idea if it fails running the code (as it could fail due command length error). All I know that if fails when I adds types, thus it is definitely not executed nor compiled as TS script.
I've just enabled error code printing
It might be a good idea to print the errors (using try..catch
), shell command exit codes in the pop-ups (or whatever you call them), as well as provide a way to log some arbitrary stuff to that pop-up (similarly how console.log()
outputs stuff to the console). Consider adding a scrollbar to the pop-ups and max height. And consider also adding a copy button too, as selecting the pop-up content (apart from double and triple-clicking to select a word or line) is quite hard.
The max cmd arg length value likely depends on the system (type, version, etc) and so I'd prefer not to constant it in the app.
The workaround I currently consider is extending formatNotificationShellExecArguments
return type by an array of objects. So the app will execute a shell script for each array item, the same way it currently works for a single object.
I believe a regular case is when the notification/unread emails count is a relative small value. So returning an object, like it works now, should work well for most cases. And for specific cases like yours, it will be possible to work around the issue by returning an array and expecting multiple shell command executions. Splitting the result to an array will be on user (no hard-coded constant on the app side).
You won't be able to use node:fs
or alike things, since the code being evaluated in a QuickJS runtime (connected as Wasm module). So there is no access to the file system or other resources. The code editor is enhanced by understanding the mail data format. There might be some imperfection in remaining globals typing, but it does good enough work.
I need to sort the emails. Can’t you sort them by mails[].Time descending please?
This is a specific need which can be handled by the custom code, so I'd prefer not to put it into the app code. Generally, adding code increases project maintenance burden, so the less is the better.
{i: i.ID, sn: i.Sender.Name, se: i.Sender.Email, s: i.Subject}
It can be an array of data vs object props, so it would keep you some space in the result string.
Looks like you hit the issue of maximum character length of arguments in a shell command. I executed the test and got E2BIG error
Well, I have just tested the following code in Node (node file.js
). It has no problem to output 999,999 chars. It might be a problem with Electron which I have never used (I used Tauri, which has much better features, smaller app size and it is possibly much more performant).
const {execSync} = require('child_process')
console.log(execSync(`/usr/bin/echo '${''.padStart(999999, '1')}' > /some/path/with/42/chars/in/length/including/filename/tmp.json`).toString())
I believe a regular case is when the notification/unread emails count is a relative small value.
Yeah. I am in the middle of creating a Node script to process mails
and I want to save mails
each adding an n
boolean property (true
means that the notification was triggered, thus no new notification will be triggered). From the backup, I purge all emails I have received yesterday and older. I could purge these in formatNotificationShellExecArguments()
as well, thus reduce the size of mails
, but in there I can’t be sure if the email has already triggered a notification or not.
Splitting the result to an array will be on user (no hard-coded constant on the app side).
Yes, splitting the array could make it work, but the performance might get worse.
the code being evaluated in a QuickJS runtime
That is good to know.
The test code you posted gives me the same error (with code=E2BIG). If I lower the size, it finishes without error.
The test code you posted gives me the same error (with code=E2BIG). If I lower the size, it finishes without error.
How did you run the code from this comment? From a terminal? What is your OS? NPM and Node version?
I tested it from a terminal using node@19.5.0
, npm@9.1.2
on Arch Linux and there is no issue with that code in itself.
It doesn't really matter which OS/node I used, since the successful exec/execSync
execution confirmed as environment/OS/node and cmd line length dependent.
Yeah, it errors out for me too, I missed the error. It does not errors out when echo
is run directly in the terminal (without Node).
Now, a soulution could be writing the command to BASH/SH file file and execute it (bash -c file.sh
) instead. Could it be easily done in Electron without node:fs
?
Clearly, there are two options for solving the issue like this. Putting the data to the file and then reading the data from it vs reading from cmd arg, or executing the shell script multiple times or the data split to the pieces. I prefer the latest option as a more isolated and more privacy-friendly option (so you put something to a file, and then the script fails for some reason, but the unencrypted data is still there in the file, or you missed removing that file in the script's logic doing testing/debugging, and the situations alike). I mean, I prefer extending formatNotificationShellExecArguments
result type by array (side benefit is that backward compatibility is respected), as written before, vs writing something unencrypted to the file system.
Could it be easily done in Electron without node:fs?
There is no issue of using node's fs
or other node modules in @electron. But the user's custom script is by design executed in an isolated environment (for this case I preferred QuickJS vs node's built-in VM module), so only pure JS-execution/computation with serializable input/output data is supported.
Clearly, there are two options for solving the issue like this. Putting the data to the file and then reading the data from it vs reading from cmd arg
Yeah, but unless you do it automatically, I cannot do it, as it might be a quite long string → E2BIG
.
or executing the shell script multiple times or the data split to the pieces.
Yes, I can do this (echo '${data[0]}' > tmp.json && echo '${data[1]}' >> tmp.json && ...
), but the performance is not ideal.
more privacy-friendly option
The most privacy-friendly and secure option would be if you added an option to enable (opt-in) individual notifications to each received email with sender and subject information directly in ElectronMail. No other way would be secure enough. That said, I understand that the notifacation server will nevertheless see the email sender and subject, but that is necessary, therefore this option should be opt-in. Yeah, I get it: it will cause additional maintenance burden, but I see no better option. It might no even require the additional database column I talked about.
vs writing something unencrypted to the file system.
As you see, even now I use echo
to write the data unencrypted to the system, just because I want this feature and I miss it very much. I trust my system enough to let the notifications contain sender and subject.
There is no issue of using node's fs or other node modules in @electron.
All I wanted to say in my previous comment that you could write the command
value to file and execute that file. If there is no problem to use node:fs
in Electron it is an option, albeit, not the best, however, it would solve the E2BIG
error.
The most privacy-friendly and secure option
I didn't talk about the most, but picked one from the two. The app is not going to persist the "notified" flag per email.
As you see, even now I use echo to write the data unencrypted to the system
Yes, and this is on you, not on the app. I don't want the app to write unencrypted data to the file system.
All I wanted to say in my previous comment that you could write the command value to file and execute that file. If there is no problem to use node:fs in Electron it is an option, albeit, not the best, however, it would solve the E2BIG error.
Same answer.
So the best option I'm ready to provide at the moment, is running exec
on the array of commands. Since you know the command length limit for your system, you, technically, will be able to form the array all command
items of which have allowed length.
Since you know the command length limit for your system
That is disputable, as during my additional tests today, I couldn’t use 131,011 chars after computing the JSON string. I could use only 83,499 chars, but even that is unstable. Currently, I am sticking to 50,000 chars. I have no idea why there is so much invariability, probably it counts some other parts of the function (or probably entire function).
Anywhere, for now I am about to ignore those messages that are outside of the 50,000 chars limit, as it is only the first run that need to parse older messages. Yes, it is not ideal algorithm, but mostly it’ll work. I’ll share it here after some tests done.
I have noticed an issue with my approach: if I get new emails while my computer is turned off, I would’t receive a new email notification at system startup. Could we run the function at ElectronMail startup?
The app downloads the missed emails first and after that triggers the notification if there are unread emails in the account. So if you have set up automatic login into the app and into the mail account, then there should be notification at the app startup with a normally small delay caused by downloading the missed emails.
It would be great to have the unread messages notification that pops up whenever there's a new mail to:
That way one won't have to neccessarily click the notification to know what the mail is about.