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

Add revertable commands #401

Closed
wants to merge 43 commits into from

Conversation

lj3954
Copy link
Contributor

@lj3954 lj3954 commented Sep 15, 2024

Pull Request

Title

Add revertable commands (competing with #384).

Type of Change

  • New feature
  • Refactoring
  • UI/UX improvement

Description

Revertable commands are added, and a database of commands which have previously been run is stored (in ~/.local/state/linutil_scripts.toml). At startup, the toml is read to determine which entries should be set to revert by default, and when a command (either normal or revert) is completed successfully, the appropriate data is written to the toml.

This is based upon the multi selection PR (#338), so the feature is fully implemented. Pressing 'r' inverts the entries shown (Revert -> Normal and vice versa). Scripts which don't support reversion have revertable = false added to their tab data entry & function normally. When multi selection is enabled, pressing 'r' does not invert currently selected options.

This is also based upon #247, since the refactoring done there could otherwise cause merge conflicts. That pull request has no downsides and should be merged before this one regardless.

Implementation within scripts matches #384; revertable scripts have a run() and revert() function, and they're run by sourcing the script and running the appropriate function. Non revertable scripts (which are not possible in #384), are executed exactly the same way they were prior to these changes. Revertable scripts were implemented by @nnyyxxxx, the commits are fully intact and can be viewed within this PR.

UI example: https://github.com/user-attachments/assets/e773d71f-c410-4a29-b38b-a95b0133f6d4

Please reference #401 (comment) and #401 (comment) regarding accusations that I've "stolen" code. TLDR; the person making this accusation was not acting in good faith whatsoever and just wanted to shut down discussion.

Testing

Commands are correctly updated in the TOML & UI on next entry, reversion of scripts works correctly

Impact

The dirs crate has been added as a dependency for linutil_core in order to fetch the preferred state directory (typically ~/.local/state) as well as possible. Another TOML is being read at startup & written to after successful command execution, which will slightly degrade performance.

Issue related to PR

Additional Information

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no errors/warnings/merge conflicts.

JustLinuxUser and others added 30 commits September 5, 2024 20:31
Fix common_script not found

Format files

Add multi-select toggle

Clear selection after execution

Refined code execution

Remove old code running method

Cleanup

Update state.rs

Add multi selection
Bug fix

Prevent floating window command screen when changing directories

cleanup

Add toml multi selection
Single command execution bug fix

Update dwmtitus-setup.sh

Fix debian depends

Update dwmtitus-setup.sh

Commit Linutil

Add hints
@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

If you want to carry on this argument please go into the community discord server please. So things can be modded.

I don't have an account with that platform, I do not like what I've heard about the hostility to privacy and security the owners of that platform have; we can move this to the discussions tab if you'd like. I don't see the purpose, though. Discussions on implementation details can be had within the PR. The discussion over the idea that I've "stolen" someone's code should end immediately, @nnyyxxxx has a very clear offer presented in front of him that would go far above and beyond and he must respond if he's concerned about my PR containing any of his code.

@nnyyxxxx

This comment was marked as resolved.

@nnyyxxxx

This comment was marked as resolved.

@Real-MullaC
Copy link
Contributor

If you want to carry on this argument please go into the community discord server please. So things can be modded.

I don't have an account with that platform, I do not like what I've heard about the hostility to privacy and security the owners of that platform have; we can move this to the discussions tab if you'd like. I don't see the purpose, though. Discussions on implementation details can be had within the PR. The discussion over the idea that I've "stolen" someone's code should end immediately, @nnyyxxxx has a very clear offer presented in front of him that would go far above and beyond and he must respond if he's concerned about my PR containing any of his code.

Please make an account as we can mod what is said and if things go to far it can be stopped easily. By our mods and very harsh bots.

@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

Respond to my proposition immediately. We're not going in a loop, this is a massive waste of time. If you don't want your code to touch my PR, I'm fully willing to do just that and you know it. If that's not your concern, immediately take back all of those accusations about me.

@nnyyxxxx

This comment was marked as resolved.

@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

Nope. We're not having this. I'll talk about UI with you after you make your decision as stated above. I don't want to discuss about "theft" any longer.

@nnyyxxxx

This comment was marked as resolved.

@nnyyxxxx

This comment was marked as resolved.

@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

You're not here in good faith. You keep accusing me of theft, but you have intentionally avoided responding to my offer to remove all traces of your code from my PR and make it depend directly upon yours, for hours. My discussion with you is over if this isn't resolved immediately.

Once again, I'll restate myself if it's not clear enough. If your wish is for your scripts not to touch my PR, I'll make my PR depend upon yours first being merged & remove all scripts you wrote from this PR. Yes, all traces. I would rebase and completely drop them. Does that solve the problem? We can discuss UI once this is resolved. If you don't accept that proposition, then kindly shut up about your code being included here, since that would completely remove it.

@nnyyxxxx

This comment was marked as resolved.

@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

nobody is accusing you of theft as stated by Adam many times.

Okay, so you're accusing me of doing something immoral. Whatever immoral thing I'm allegedly doing, would my offer, as stated above, cause you to discontinue these accusations? I would like a response in 10 minutes time, I'm willing to immediately work on said changes to address your concerns. Then, we can debate over the UI. Alright? Otherwise, you must not be interested in anything other than throwing horrible and baseless accusations upon me, and this discussion will cease.

@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

This commit was worked on earlier, don't mind it. Not that I should apologize to you for making a commit with my own code regardless.

@nnyyxxxx

This comment was marked as resolved.

@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

So you really don't care about your code being present here, and you just wanted to use that accusation to shut down discussion. Truly pathetic, that has no place in a civilized discussion. I hope you can reconsider using such a strategy in a discussion.

@adamperkowski, do you stand by this? I would hope not. I do understand that you might take his side in the discussion with his code indeed present here, although I do disagree over whether it's really problematic, given the credit already present. But now, with him very clearly trying to sweep my offer to remove all traces of his code under the rug, you should be able to see how he's not interested in a good faith discussion.

Now that we have this nonsense over with, I'd like to discuss over the merits of the 2 UIs against each other. Would anyone who's interested in discussing with good faith be willing to participate?

@adamperkowski
Copy link
Collaborator

So you really don't care about your code being present here, and you just wanted to use that accusation to shut down discussion. Truly pathetic, that has no place in a civilized discussion. I hope you can reconsider using such a strategy in a discussion. @adamperkowski, do you stand by this?

Don't @ me just to provoke me and nyx. Read my comments first (especially this one: #401 (comment)), then we can have a normal, civilized discussion over on Discord. This is my last comment on this PR.

@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

So you really don't care about your code being present here, and you just wanted to use that accusation to shut down discussion. Truly pathetic, that has no place in a civilized discussion. I hope you can reconsider using such a strategy in a discussion. @adamperkowski, do you stand by this?

Don't @ me just to provoke me and nyx. Read my comments first (especially this one: #401 (comment)), then we can have a normal, civilized discussion over on Discord. This is my last comment on this PR.

I offered to address these concerns, despite the fact that I disagree over whether they're reasonable. The person in question, though, rejected my offer by completely disregarding it, for hours, so obviously he's not acting in good faith. I'm not trying to provoke you, I'd just hope you're not interested in standing by such a disgraceful attempt at shutting down discussion. Discussion over the merits of the UI approaches should proceed, without accusations that one isn't willing to stand by. I hope you can agree to resolve those talks, so that we can proceed in a civilized manner.

We can refer to examples of positive and negative behaviour in the recently merged code of conduct to ensure we never repeat such a mistake.

Accepting responsibility and apologizing to those affected by our mistakes, and learning from the experience

Trolling, insulting or derogatory comments, and personal or political attacks

@ChrisTitusTech
Copy link
Owner

Let's keep it civil there is no need for finger pointing or name calling. We are all simply trying to make the best program possible. Constructive criticism of their code is good (Example. This PR has performance issues, I'd recommend X or Y).
Attacking a person with name calling is not. (Example. You are fat).

I also like to credit anyone that contributes code. So if anyone does use someone else code just @ the person or reference the PR the code was taken from letting them know that you incorporated the code or the idea from that PR in the project.

Thanks for understanding lets keep this a fun project to work on.

@lj3954
Copy link
Contributor Author

lj3954 commented Sep 15, 2024

I also like to credit anyone that contributes code. So if anyone does use someone else code just @ the person or reference the PR the code was taken from letting them know that you incorporated the code or the idea from that PR in the project.

I've done exactly this, and offered to go way above and beyond when someone suggested that any of their code being within my PR would go against their wishes. I hope for more people to enter this discussion now that these comments have been resolved, in order to discuss in a civilized manner whether my solution or the solution presented in #384 is preferable.

@lj3954 lj3954 mentioned this pull request Nov 7, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uninstall? undo the changes ? Revert changes
7 participants