bobafetthotmail / refind-theme-regular

http://munlik.deviantart.com/art/rEFInd-theme-512091944
Other
783 stars 76 forks source link

better install steps #7

Closed noraj closed 5 years ago

noraj commented 5 years ago

DONE

TODO

bobafetthotmail commented 5 years ago

I'm mostly ok with this, but sudoedit does not work properly on my OpenSUSE, I'm getting "editing files in a writable directory is not permitted" error.

As you can see in the instructions in the below link, I can fix it by changing the configuration but this is supposed to be a tutorial that works everywhere by default. https://www.smarinov.com/weblog/sudoedit-not-working/

So I'll ask you to either reverse that change or find another way that works more reliably in different distros.

noraj commented 5 years ago

@bobafetthotmail nano is not installed on all linux distro. The only POSIX editor present on all unix system is vi. sudoedit is part of sudo ann will work with the default editor vi or with one configured in EDITOR/VISUAL environment variable.

The bug you add was due to upgrading sudo from an old version to a newer one where sudoedit_follow was required. Most distro will auto-merge your actual config file with the newer or create a save version of the new config file (like .pacsave on ArchLinux). So it is not a bug bug, just you that forgot to update your config file after a major update of a package. sudoedit will works on any Linux distro where you have sudo installed either with an old or latest version. Only people who has upgraded from a very old to a newer version at some point AND forgot to update their config file will have this problem.

TL;DR: sudoedit will works pretty everywhere. The other possibility is to use sudo vi but lot of people don't know how to use vi and that is the same problem as using sudo anyeditor rather sand the safest sudoedit.

Tell me if you prefer I let sudoedit or if you prefer I put sudo vi? It is also possible to the convention $ for standard user permissions and # for root user permissions and so removing the need to write sudo before commands and letting people choose the way they will elevate their privileges.

bobafetthotmail commented 5 years ago

Can reproduce the issue in a new install too, it's not an old config issue.

I also have no environmental variables to set default editor as they set default editor though XDG system, so I can open my default editor with xdg-open /path/to/text but of course this is opened as my user, doing it with sudo uses root's XDG defaults which seems to be Lynx for some reason (yes I know it is a browser, not an editor, I have no idea why it's picked)

In my experience, distros have different default configs.

That said, I would remove the first sudo nano blablabla, people using the script will be selecting the size they want during the installation, no need to edit config manually for them.

Then at the second one I would add a few more lines with gedit, kwrite, kate, pluma, mousepad, leafpad and others that are most common (i.e. default installed with GNOME/KDE/XFCE/LXDE).

If the user has installed a custom editor and wants to do stuff manually I'll assume he is smart enough to edit the file with his own editor of choice.

bobafetthotmail commented 5 years ago

Ok, you intend to alter the install script too? No rush, I'm just asking because I see the "TODO" above. I just don't want to merge this until you are done.

noraj commented 5 years ago

as I changed the path in the conf the script need to be updated. But when I saw the ugly sed I didn't want to touch them :rofl: image I prefer to let you change your own script, as an advice you should use more variables rather than hard-coded path, change would be easier.

bobafetthotmail commented 5 years ago

Are you seriously telling me you can't add a "themes\/" before every "refind-theme-regular" because it's too "ugly" for you? And you're even giving me "advice"?

I could have accepted a "I don't know how to do that" answer, but I'm not going to change things for people with this attitude.

noraj commented 5 years ago

@bobafetthotmail Don't be mad :( I'm sorry you miss-interpreted my thoughts.

The themes/ is not because of ugliness, it is because other themes are deployed in themes/ so yours ends in the root folder, that non-standard.

I said the big sed was ugly, that's all, it's true that it is hardly readable. I hunbly gave you an advice to use more variables in order to keep the script more readable.

So yeah I didn't modified the script by laziness when I saw how it is written, I'm sorry that hurt you when I said the script was not very well written.

noraj commented 5 years ago

I finally made the changes https://github.com/noraj/refind-theme-regular/commits/master

bobafetthotmail commented 5 years ago

So yeah I didn't modified the script by laziness when I saw how it is written, I'm sorry that hurt you when I said the script was not very well written.

I still don't get what is wrong with hardcoded paths in sed if the script does not need to change them on runtime, but that's beside the point.

You come here and open a PR about a change that is mostly cosmetic (for me at least), and I'm ok with that.

But then you bash on me because a simple and dirty install script isn't up to your liking (again, for issues that are mostly cosmetic because the script works and altering it takes 20 seconds flat)? Sorry what?

And you even admit you refuse to alter it because you "want me to learn to write better scripts"?

You really need to be more respectful and keep these thoughts to yourself, especially if you are submitting code to someone else.

What if I made it and wanted to invest as little time in a simple thing like that?

What if it was actually made and then modified by other contributors of lesser skill level (see other PRs) and I don't feel like reworking it because I wouldn't have done much different for a quick and dirty install script?

I finally made the changes https://github.com/noraj/refind-theme-regular/commits/master

If you want to open another PR I can accept it.

While I still don't like your attitude I'm not rejecting self-sufficient contributions (i.e. I don't have to fix stuff for your PR to work).

But I can live without too, so that's your choice.

noraj commented 5 years ago

I still don't get what is wrong with hardcoded paths in sed if the script does not need to change them on runtime, but that's beside the point.

image

I used a variable to shorten the sed command and make it more readable, also changing sed delimiter to avoid using a backslash in path.

You come here and open a PR about a change that is mostly cosmetic (for me at least), and I'm ok with that.

It is not really cosmetic: themes is for standard as all other themes are deploying in themes. And readme changes with sudo/sudoedit is for safer steps.

But then you bash on me because a simple and dirty install script isn't up to your liking (again, for issues that are mostly cosmetic because the script works and altering it takes 20 seconds flat)? Sorry what?

Take a breath and just relax. Don't be mad just because someone said the syntax of one of your command is not optimal or not readable or not very well written. Just admit it when it is true. That's not a big deal to accept criticisms. Also I said I was sorry it hurt you, I didn't know you were so sensitive/emotional. May be the fact I'm not an english native speaker make my comment more rude.

And you even admit you refuse to alter it because you "want me to learn to write better scripts"?

Not correct, I don't want any from you. I just said that seeing the big sed mess discouraged me to modify the script.

You really need to be more respectful and keep these thoughts to yourself, especially if you are submitting code to someone else.

In the first place I didn't see ugly sed as an insult or something highly unrespectful.

I said:

But when I saw the ugly sed I didn't want to touch them :rofl:

I don't have to fix stuff for your PR to work

Yep but working together is the base of OSS.

As you close my issue I just forked it and made the change myself on my fork. Hopefully it is still open-source and all changes I made are available here : https://github.com/noraj/refind-theme-regular/commits/master so feel free to copy them in your fork.

Honestly I was surprised when I saw you reacted so badly:

Are you seriously telling me you can't add a "themes/" before every "refind-theme-regular" because it's too "ugly" for you? And you're even giving me "advice"?

I could have accepted a "I don't know how to do that" answer, but I'm not going to change things for people with this attitude.

Again I'm sorry to see you misunderstood my means all along and I'm sorry you made such a drama just for me saying ugly sed. It was not even targeting you. I'm not angry, or disrespectful and I still love your dark theme, your availability and open mind so just take a big breath and forget that :heart: :heart: :heart:

bobafetthotmail commented 5 years ago

It is not really cosmetic:

Yes it is. Regardless of where you place the theme you need to add its path in the config file, and most people won't have more than one theme installed anyway.

If rEFInd now has a standard theme folder it will look into without adding configs, then yes it's not cosmetic anymore.

Don't be mad just because someone said the syntax of one of your command is not optimal or not readable or not very well written.

I'm mad because you tried to school me because of a quick and dirty install script. Yes it's ugly. I don't care. I didn't even write that script in the first place. I only checked that it worked as intended and merged the PR.

feel free to copy them in your fork.

As I said I don't need this change for myself, so it won't happen. Heck I didn't need the install script at all either, it was someone else that contributed it.

I will accept PRs from you or from others as long as they work and don't require me to change stuff for you, that's all.