ChrisTitusTech / linutil

Chris Titus Tech's Linux Toolbox - Linutil is a distro-agnostic toolbox designed to simplify everyday Linux tasks.
https://christitus.com
MIT License
1.75k stars 158 forks source link

feat: Use TOML data to add tabs to UI. #153

Closed lj3954 closed 3 weeks ago

lj3954 commented 1 month ago

Pull Request

Title

Use JSON data to add UI entries

Type of Change

Description

(These additions are built on top of #136) Replace the need to manually update the tablist in the Rust codebase upon script addition. Instead, contributors only have to create a JSON file under the same name as the script with everything that's necessary.

Most scripts will only need to add a ui_path field, which specifies the path in the UI (as an array of strings). The first value is the tab, the last is the entry name, and anything in between are directories within the UI.

(Optional) Descriptions can be added to the JSON, but they're currently unused. In the future, they can be offered to the user before they run the command, similar to the current preview window.

This also addresses #152, by offering an optional 'preconditions' field in the JSON data. This field allows scripts to check system files or environment variables to ensure they either include or exclude certain specified values. This allows linutil to hide scripts which are unsupported on certain desktop environments, display servers, or distributions. The xrandr monitor control scripts are already affected by this, and there are other proposed scripts which would also be able to make use of this.

"Raw" commands previously within list.rs (or tabs.rs in #136) are now implemented through JSON files which aren't named the same as a script. On top of everything mentioned previously, a 'command' field is added, which contains a String.

This PR has been changed from its original proposal, due to feedback. Now, TOML is used rather than JSON, for easier readability, and each tab has its own file containing all necessary information (full tabs.toml file in the commands directory points towards directories containing individual tabs). The individual tab files contain a name field and 'data' array. Each entry in the array contains a name, optional preconditions and description (as mentioned above), and can contain either a "script" field, pointing to a script (starting at the tab directory), a "command" field, with a raw command, or an "entries" field, containing another array of entries.

Testing

I've added JSONs for all the preexisting scripts, and they render as expected in the TUI. All preexisting scripts work correctly.

Impact

Contributions should be significantly easier, especially for those who don't know or want to read and modify Rust code.

Issue related to PR

[What issue/discussion is related to this PR (if any)]

Checklist

afonsofrancof commented 1 month ago

I really like this. To unify things, wouldn't it be better for the raw commands,to also have their own .sh file? The file would just curl the url, and then run it. That way everything would be treated the same way, and if we ever wanted to add more stuff to that command, it would be easier. It would also reduce the logic of checking if the JSON file has a command field or not to choose what to run. What do you think?

afonsofrancof commented 1 month ago

It would make it so we could add some prints of "Running X tweak..." and "Finished..." and any other things we want to the raw commands

afonsofrancof commented 1 month ago

Also, if we end up using one JSON file for each command, we should think of maybe embedding the JSON in the script itself.

I like this, but haven't found a good way to embed it. Maybe we could have it as a comment in the beginning of the script, but I really dislike this approach.

Just had this idea and wanted to leave it here in case someone thinks of a good implementation.

lj3954 commented 1 month ago

Tomorrow I'm going to rework this, the JSON per is probably the wrong approach. The main commands directory should instead have a 'tabs' file, containing a list of tab names, each pointing to another file containing the full data needed for a tab. I'm also probably going to change the format to TOML, since large arrays are simpler in it as compared to JSON.

afonsofrancof commented 4 weeks ago

We could just make the directory structure directly map to the commands structure inside the tool. Then, if a directory inside the commands directory is named "applications-setup", we could remove the dashes and capitalize the first letter, so it would show up as "Applications Setup" inside the tool.

This way, everything added to the commands' directory would automatically be added to the tool, which would be even easier for everyone to contribute.

We could then optionally have a TOML file inside each directory with rules about the commands inside it. This file would be optional, only existing if we want rules to be set to those commands.

Basically #140 with optional TOML restrictions for the commands.

lj3954 commented 4 weeks ago

I disagree. First, you're not always going to be able to convert camelcase names into what you want. For example, if an item needs parens or some other non-alphanumeric character, you won't be able to account for that, and adding such characters to a filename is generally a poor choice. We also have the problem of scripts which are dependencies, such as common-script, and one for the monitor control scripts.

I think that would be a nightmare waiting to happen.

lj3954 commented 4 weeks ago

These changes should make contributions easier. https://github.com/ChrisTitusTech/linutil/pull/153/commits/d4489648b4ea7d17f38ebeb8577b79e37bd83ef6 makes sourcing other files work exactly how it would for a standalone script, by using the directory in which the script is contained rather than the root of the temporary directory.

afonsofrancof commented 4 weeks ago

Ok, I understand the shortcomings. Makes sense

afonsofrancof commented 4 weeks ago

Is this PR finished @lj3954 ?

ChrisTitusTech commented 3 weeks ago

Love the changes, just need to update it to accommedate the earlier #136 merge.

lj3954 commented 3 weeks ago

Tomorrow I'll work on separating search/filter logic from the AppState (building on ideas from #120). This one is ready to merge.

afonsofrancof commented 3 weeks ago

@lj3954 Yesterday on stream, I asked Chris not to delete the search.rs file so it could be used later to move search out of state.rs. I also have free time in case you need me to do it or if you need help doing it.