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

[enh] Implement 'yunohost log show last' to display the last log file. #1805

Merged
merged 4 commits into from May 18, 2024

Conversation

Abperron
Copy link

@Abperron Abperron commented Mar 21, 2024

Resolves YunoHost/issues#2047

The problem

As said in YunoHost/issues#2047, yunohost log show is annoying to use because it's required to know the name of the log file.

Solution

The path argument can be replaced by the "magic" keywords last or last-X to display the last or last but X log file (last-0 means last).

log_show now uses the log_list function.

We didn't internationalize the error message if 'X' is too big because it can only appear in the CLI for now, which seems not internationalized either.

PR Status

Done.

How to test

  • Run yunohost log show last or yunohost log show last-1.

src/log.py Show resolved Hide resolved
@Psycojoker
Copy link
Member

I don't understand why you aren't using CLI arguments like "--last/-l" and "--number-of-logs/-N" and parsing a string instead, is there a reason for that?

@manor-tile
Copy link
Contributor

We did this because the issue seemed to propose to do this, and it makes sense because the path argument already means "which log file to show?".

I also think it's easier to read, remember and write, compare:

yunohost log show last-1
yunohost log show -l -N 1
yunohost log show --last --number-of-logs 1

On the other side using dedicated CLI arguments as your propose is more standard and avoids using a regex, so there is a trade-off between our solution and yours and we are not sure about which is best.

@manor-tile
Copy link
Contributor

Would an alternative pull request using dedicated CLI arguments be appreciated? Is there something else we could do to enhance our pull request or resolve the related issue ?

@kay0u
Copy link
Member

kay0u commented Apr 4, 2024

Sorry about the response time, we just need volunteers to test the code and press the green button 🙂

@alexAubin
Copy link
Member

Cheers and sorry for the delay, bonus point for the regex thing, the named "position" group thing is neat 👍 👌

@alexAubin alexAubin merged commit 48c6734 into YunoHost:dev May 18, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants