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

functionize most actions, add main(), use [ thing ] to check #79

Closed
wants to merge 1 commit into from
Closed

functionize most actions, add main(), use [ thing ] to check #79

wants to merge 1 commit into from

Conversation

apprehensions
Copy link

original commit description:

added --verbose, -s, change HELPSTRING to comply with it, format xdg missing variable to comply with main()'s style of messages, shorten check_if_file_exists, organize functions & variables, added placeholder for unalias -a for future maintainer message, add comments

to be honest, i don't know what [ thing ] should be called.
i can still change my commit messages with --amend

notes file:

  • added --verbose flag
  • added -s flag, shortcut for --skip-ok
  • change HELPSTRING to comply with above [1]
  • make checking for jq display red colors to look scary
  • remove extra new line in HELPSTRING
    • to be honest, i really wanted to change the HELPSTRING to something simple:
     usage: xdg-ninja [--help|-h] [--no-skip-ok|-v|--verbose] [--skip-ok]
    
    or:
     usage: xdg-ninja [OPTION]
     	--help, -h                    display this help and exit
     	--no-skip-ok, -v, --verbose   display messages for all files checked
     	--skip-ok                     do not display files that do not exist (default)
    
    IMO, a help message should not look like the default, since mostly all programs follow the same help message, this one is like an essay lol, unless the maintainers want to keep the original help message:
     <color>xdg-ninja<color>
     <bold>Check your $HOME for unwanted files.<reset>
    
     -h, --help                   This help menu
     -v, --no-skip-ok, --verbose  Display messages for all files checked
     -s, --skip-ok,               Don't display files that do not exist (default) 
    
  • changed xdg warning format from
<cyan color>
The $XDG_STATE_HOME environment variable is not set, make sure to add it to your shell's configuration before setting any of the other environment variables!
    ⤷ The recommended value is: $HOME/.local/state
<cyan color>

to

<color 7> The <cyan color>$XDG_STATE_HOME<color 7> environment variable is not set, make sure to add it toyour shell's configuration before setting any of the other environment variables!
	<color 7 bold> The recommended value is <cyan color>$HOME/.local/state<cyan color>
  • change the message if a markdown renderer is not found.
  • in check_if_file_exists(), FILE_PATH is not in double quotes, i'm not sure why but i'm not touching it.
  • as [ -e "$FILEPATH] already returns, i assume you are simply reversing it, an easier way to do this is [ ! -e "$FILEPATH" ], it would simply already return the return value by itself.
  • added some comments wherever neccesary
    • PLEASE make my comments better, i do not know how to explain.
  • change check_programs to be safer(?). (my source? i've read it somewhere.)
  • shouldn't we use the tput tool for colors?
  • organize functions by usage
  • seperate functions
  • seperate variables
  • rename do_check_programs to check_programs
  • add main() to handle everything(?)
  • move unalias -a to the top because i still don't know what it does.

  • SC2015 does not make sense. it is exactly what i want it to do, In this case, if A is true but B is false, C will run. and Ignore this warning when you actually do intend to run C when either A or B fails., i will revert it if needed.
  • the README.md should probably comply with [1]
  • why dont we just use image resources from a file in this repository?
  • currently,
     void-live[~/xdg-ninja]612ms% shellcheck xdg-ninja.sh
     void-live[~/xdg-ninja]43ms%
    

i made this PR one commit only, why? many of the changes i've added, if having completely seperate commits and wanting only one of the commits, will probably have merge conflicts.

added --verbose, -s, change HELPSTRING to comply with it, format xdg missing variable to comply with main()'s style of messages, shorten check_if_file_exists, organize functions & variables, added placeholder for unalias -a for future maintainer message, add comments
@apprehensions
Copy link
Author

personally, i still find it quite difficult to have to separate every single action.
i can still manage it but see the description for what i had meant.

@b3nj5m1n
Copy link
Owner

b3nj5m1n commented Jun 5, 2022

This has the same issues as your previous PR, you're still rewriting half the project. I said I'd think about merging two of the changes you made, most of this has nothing to do with those.

@b3nj5m1n b3nj5m1n closed this Jun 5, 2022
@apprehensions
Copy link
Author

This has the same issues as your previous PR, you're still rewriting half the project. I said I'd think about merging two of the changes you made, most of this has nothing to do with those.

What?? What do you exactly want me to do then?

i made this PR one commit only, why? many of the changes i've added, if having completely seperate commits and wanting only one of the commits, will probably have merge conflicts.

Also, I had done nothing with the actual core yet, I am simply reorganizing the script.

@apprehensions
Copy link
Author

You know what? Fine, I will just do one PR per change.

@apprehensions
Copy link
Author

. I said I'd think about merging two of the changes you made

Please tell me exactly what you are interested in so I can focus on those, it had seemed like you hadn't read the description of this PR.

@b3nj5m1n
Copy link
Owner

b3nj5m1n commented Jun 5, 2022

Please tell me exactly what you are interested in so I can focus on those

Mainly it's the part that checks for the existence of the xdg env vars.

@apprehensions
Copy link
Author

Please tell me exactly what you are interested in so I can focus on those

Mainly it's the part that checks for the existence of the xdg env vars.

In that case, I will mainly focus on making if statements better.

@apprehensions
Copy link
Author

apprehensions commented Jun 5, 2022

before i open another PR and maybe get rejected again, i will just ask if these following features will get accepted:

  • minimize all if statements to [ thing ]
  • use passed variable $MDRENDERER for markdown renderer if passed to script
  • use variable $MDRENDERER for the markdown renderer
  • adding -s for shortcut of --skip-ok
  • adding --verbose for extended of --no-skip-ok
  • change jq message to red
  • minimize check_file by using the print() function used in this PR and a case for $MDRENDERER

personally, it would not make sense to have the simpler xdg env vars look they are while the other if statements dont look the same

@b3nj5m1n
Copy link
Owner

b3nj5m1n commented Jun 5, 2022

No.

@apprehensions
Copy link
Author

In that case the scripting style will be very unorganized so it shall stay the same..

@FranzXaver
Copy link
Contributor

@wael444 with all due respect, you might be better off maintaining your own fork. I don't really understand the pressure you are making here. Nobody is forcing you to contribute?

@apprehensions
Copy link
Author

@wael444 with all due respect, you might be better off maintaining your own fork. I don't really understand the pressure you are making here. Nobody is forcing you to contribute?

why even bother making my own fork if no one will use it other than me?
i have no idea how to use cabal either. if i wanted to make my own fork I'd start from scratch.
the only reason i have been trying to contribute is to just make the code more cleaner, that is merely all i wanted to do.

@b3nj5m1n b3nj5m1n mentioned this pull request Jun 5, 2022
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.

None yet

3 participants