Feat/current alternative formating

[rafaltrojniak]

I wanted to provide JSON output for simple CLI commands, like current, activities or categories.

In attached MR you can see my proposition of implementation using MVC approach.

[mwilck]

Thanks for the PR. Unfortunately I have to say I'm not convinced.

We have the "export" functionality for choosing different output formats. It would be straightforward to add JSON as a new export format. Pushing it into the main CLI, as here, would make JSON special compared to CSV and XML. I see no reason for doing that. It makes little sense from the user perspective, either. If the user wants XML output, she needs to run "export", and for JSON she must use "list --format", that's inconsistent.

Moreover, putting this functionality into hamster-cli prevents json from being used in other contexts. JSON format would probably good-to-have in the GUI too, and perhaps even for foreign clients like the GNOME extension. I am no friend of changes that favor one UI over the other.

This said, a JSON export format would be highly welcome. And feel invited to convince me that I'm wrong and that the way you implemented this was exactly what we need.

[matthijskooijman]

I haven't looked at the code yet, but I would agree that extending the current export command would be favorable over adding a new command (at least from a UI POV).

JSON format would probably good-to-have in the GUI too, and perhaps even for foreign clients like the GNOME extension.

How would you see this working? The GUI does not currently have any CSV or XML features either, right? And the DBus API actually already supports JSON format for facts:

hamster/src/hamster-service.py

Line 319 in 21facca

def GetFactsJSON(self, dbus_range, search_terms):

Re-reading the original PR description, I believe the intent is to add JSON formatting as a machine-readable for various command, so not just for export. IOW, adding a JSON export format would not solve the entire problem that (I think) this PR is trying to solve. So maybe both of these could co-exist: having a json export format, and allowing --format json on other commands too?

[rafaltrojniak]

@mwilck my intention was to add JSON/XML/TSV or any other programatic output format to small CLI commands like current.
What is the proposed way of doing that?

Currently the output format is flagged only for export command, other commands do not have any way of changing format.

[rafaltrojniak]

So trying to guess your expectations, I have two options to get JSON output from current endpoint:

1. • Try to inline determine fomat and just put one form or the another - https://gist.github.com/rafaltrojniak/95a8090341c195d8f8624fb8bbede70e . That's pretty ugly
2. • Try to utilize reports module, implement JSON and text report format. Then use reports to generate single fact report in given format.

What's your proposed solution to do that ?

[mwilck]

How would you see this working? The GUI does not currently have any CSV or XML features either, right? And the DBus API actually already supports JSON format for facts:

It has the export functionality, that's it. I haven't thought this through. The reasoning was more like "if we implement this for CLI only today, we may notice some time later that it could come in handy elsewhere, too".

Re-reading the original PR description, I believe the intent is to add JSON formatting as a machine-readable for various command, so not just for export.

hamster export xml writes to stdout. This would work just fine for pipelines.

[mwilck]

@mwilck my intention was to add JSON/XML/TSV or any other programatic output format to small CLI commands like current.
What is the proposed way of doing that?

current could be approximated with hamster export xml -1:

$ hamster export xml -1|xmllint --pretty 1 -
<?xml version="1.0"?>
<activities>
  <activity name="hamster" start_time="2020-06-05 21:49" end_time="None" duration_minutes="2.0" category="Develop" description="" tags=""/>
</activities>

This is because "-1" means "1 minute ago" to hamster. I have to admit this doesn't always work; if the current activity started less than 1 minute ago, it may print the previous activity, too.

I understand your idea now better. I'm not against a generic --format switch, but I'd like to see the core code shared between the CLI output and the export functionality. IOW, implement "export json" first, and make the code reusable for your CLI needs.

The most generic solution would leverage all the existing export formatters, and allow to use any of them for CLI output.

Next, talking about JOSN, as @matthijskooijman already hinted at, users might expect JSON input to work, too 😁 ... but let's focus on output first.

[rafaltrojniak]

@mwilck Thanks for constructive feedback, now I think we can get to something.

Please see MR commits now , I'm using reports.simple to format the output in already existing formats.

I've added --format option that help with format change. Because basing this parameter as positional argument would require changing each action, I've passed it through private variable.

Implementation of JSON exporter is proposed on #617

[rafaltrojniak]

@mwilck @matthijskooijman is there anything I can do to help you with that MR further ?

[mwilck]

Sorry I had no time for hamster lately... so this is on top of #617?

[rafaltrojniak]

@mwilck That's a start. If you accept that approach I would suggest to continue accept it as is, then I'll modify #617 to cope with it.