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

Make JSON output more consistent across commands #547

Open
ajanvrin opened this issue Jan 18, 2025 · 5 comments
Open

Make JSON output more consistent across commands #547

ajanvrin opened this issue Jan 18, 2025 · 5 comments

Comments

@ajanvrin
Copy link
Contributor

Context

I don't know for other users but my main usage of himalaya will be programmatic: I'm going to integrate himalaya with Python or Powershell scripts to handle daily email sifting and sorting and perhaps sometimes answer drafting (AI tools like ollama come to mind).
Long story short, I will almost exclusively use the JSON output and only use the nice tables for testing and debugging purposes.

Possibility for improvement

Currently, himalaya does not really return JSON upon imperative operations such as folder creation or deletion, or rather, it serializes to JSON the message that would have been printed to the user. At least that's what it does upon success.
It would, in my humble opinion, be nicer if it returned a boolean assorted to a message (or even just a boolean).

On failure however, himalaya does not return JSON, it simply errors out, which is not very nice for pipelining purposes.
The boolean assorted with a message would be quite preferable.

As a side note, currently creating a folder twice first yields a success then an error. Maybe yielding two successes "The folder has been created" then "The folder already exists" might be better.
Going further in that line of thought: currently add is an alias for create. I've recently gone through the pain of configuring a Linux firewall with netfilter, and it turns out it has interesting semantics for create and add. Maybe adopting that semantic would be unnecessarily fancy, but in netfilter, add and create do the same thing when the firewall/rule does not exist, but when it already exists, add does not yield an error (because after all the table/rule already exists), and create yields an error because the table/rule could not be created.

As a side side note, the tone of the messages in JSON might be slightly less enthusiastic than the console output and drop the exclamation mark at the end ¯\(ツ)

Examples

Folder creation

Current behavior:

himalaya --output json folders create "Folders/potato/banana"
"Folder Folders/potato/banana successfully created!\n"

Preferred behavior:

himalaya --output json folders create "Folders/potato/banana"
{"result": true, "message": "Folder Folders/potato/banana successfully created"}

Folder creation when the folder already exists

Current behavior:

himalaya --output json folders create "Folders/potato/banana"
Error:
   0: cannot create IMAP mailbox
   1: cannot resolve IMAP task
   2: unexpected NO response: a mailbox with that name already exists

Note: Run with --debug to enable logs with spantrace.
Note: Run with --trace to enable verbose logs with backtrace.

Preferred behavior:

himalaya --output json folders create "Folders/potato/banana"
{"result": true, "message": "Folder Folders/potato/banana already exists"}

If the same semantics as netfilter's add and create are to be adopted

add

himalaya --output json folders add "Folders/potato/banana"
{"result": true, "message": "Folder Folders/potato/banana already exists"}

create

himalaya --output json folders create "Folders/potato/banana"
{"result": false, "message": "Folder Folders/potato/banana already exists"}

I'd actually argue against implementing netfilter's behavior for simplicity's sake

Just having a "true" because the folder is already there, regardless of whether add or create was used, would be mighty fine, in my humble opinion.

Folder deletion

Current behavior:

himalaya --output json folders delete --yes "Folders/potato/banana"
"Folder Folders/potato/banana successfully deleted!\n"

Preferred behavior:

himalaya --output json folders delete --yes "Folders/potato/banana"
{"result": true, "message": "Folder Folders/potato/banana successfully deleted"}

Folder deletion when the folder does not exist

himalaya --output json folders delete --yes "Folders/potato/banana"
Error:
   0: cannot delete IMAP mailbox
   1: cannot resolve IMAP task
   2: unexpected NO response: no such mailbox

Note: Run with --debug to enable logs with spantrace.
Note: Run with --trace to enable verbose logs with backtrace.

Preferred behavior:

himalaya --output json folders delete --yes "Folders/potato/banana"
{"result": false, "message": "No such folder"}

Addendum

I've yet to discover and use all of himalaya's features, but this opinion on JSON output is probably transferable to other functionalities too, not just folder creation/deletion.

@ajanvrin
Copy link
Contributor Author

ajanvrin commented Jan 18, 2025

And actually, the behavior that would be most preferable in my humble opinion, is to not actually return a message at all, just a boolean indicating whether the operation was successful or not.
And not implement netfilter's semantics, and simply return true if the folder could not be created because it already exists (returning false would be misleading in that case).
Same for delete, by the way, if the folder could not be deleted because it does not exist, himalaya should just return true imho. That's where the assorted message would be useful: disambiguating those edge cases, but if such a choice is made, then always returning a message along with the boolean even in the simplest cases would keep the output structure consistent.

@soywod
Copy link
Member

soywod commented Jan 18, 2025

First of all, thanks for your detailed explanation.

Long story short, I will almost exclusively use the JSON output and only use the nice tables for testing and debugging purposes.

I only had few feedback about the JSON feature (which I barely use myself), so I would be glad to have one from a potential real user of that feature.

This issue is definitely related to #539. I am currently not satisfied with the way Himalaya CLI manages output. To summarize (including your feedback):

  • stdout is used to display messages, tables and JSON output
  • stderr is used for error messages and stacktrace (no JSON handled at all), as well for logs

The initial idea I had was to:

  • keep stdout for app output, which includes regular messages, tables, JSON as well as error messages
  • keep stderr for logs only
  • to determine if the output on stdout is regular data or error, the exit code should be enough

To complete with what you proposed, when --output json is set, all JSON output (data and error) should go to stdout.

It would, in my humble opinion, be nicer if it returned a boolean assorted to a message (or even just a boolean).

I'm afraid it would conflict with the exit status, which is IMHO way enough to determine if sth went wrong or not. The boolean can be easily simulated with exit_code == 0. But I definitely agree on always returning a JSON object for consistency.

Regarding add and create, I have no strong opinion. I am not against having 2 commands with different exit code. Note that if you mkdir on an existing folder you get a status code 1 which is considered an error.

For all the rest I agree.

The first thing would be to tackle this stdout/stderr thing, then we can move on JSON output.

@ajanvrin
Copy link
Contributor Author

ajanvrin commented Feb 9, 2025

Hello!

Sorry for not answering sooner.

To complete with what you proposed, when --output json is set, all JSON output (data and error) should go to stdout.

Yes, and anything else intended for humans can still go to stderr

I'm afraid it would conflict with the exit status, which is IMHO way enough to determine if sth went wrong or not. The boolean can be easily simulated with exit_code == 0. But I definitely agree on always returning a JSON object for consistency.

Yes, that's a sensible approach, the boolean can be the exit code, and the json can contain the reason.
however, please ensure himalaya does not abort execution upon encountering an error.
I am not sure what the current behavior is, but himalaya should keep doing what's it going as long as possible.
That might already be the case actually.

Regarding add and create, I have no strong opinion. I am not against having 2 commands with different exit code. Note that if you mkdir on an existing folder you get a status code 1 which is considered an error.

Me neither, all options are fine, my only opinion on the subject would be to try to keep things simple.

The first thing would be to tackle this stdout/stderr thing, then we can move on JSON output.

Agree

As far as I'm concerned, this issue no longer affects my own use of himalaya, feel free to keep it around or close it as you see fit without asking.

@soywod
Copy link
Member

soywod commented Feb 17, 2025

Let's keep the issue for a more consistent JSON output. Tightly related to #539.

@soywod
Copy link
Member

soywod commented Feb 18, 2025

I extend the scope of this task to all commands. JSON output should be consistent across all commands.

@soywod soywod changed the title Make creation/deletion operations with -o json actually return JSON Make JSON output more consistent across commands Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants