Function "cp_n_chown_n_chmod": Some things to improve

[keks24]

Hi Alexey,

this is your first pull-request! \o/

Below, I explain to you what I did to create this pull-request:

1. Create a fork of your repository to my account, since I have no direct access to your repository
2. Clone my fork: git clone [email protected]:keks24/dotfiles-1.git
3. Make changes
4. Push them to my fork
5. Create a pull-request, comparing my changes with the master branch of your repository (compare across forks: https://github.com/halcon74/dotfiles/compare):

[base repository: halcon74/dotfiles] [base: master] <- [head repository: keks24/dotfiles-1] [compare: master]

6. Discuss about this pull-request or not
7. Personal cleanup: Archive the fork after the pull-request has been adapted or declined

This is just a suggestion. You as an owner can decide, to adapt it or not

I also left some in-line comments, which you might resolve.

[halcon74]

Hi Ramon,

Thanks a lot for your pull-request and for the explanations!

I have a feeling that no matter how I like to continue working with mercurial, yes, I have to learn git more.

Creating these repositories here on GitHub was the first step. As all my local repositories are in mercurial, and I'm still waiting a response for my hosting request on Heptapod, I've done so far all my commits here via web interface, and only some of them are the same as in my local mercurial... a mess. Now, for working on your pull-request, I'll have to create a local git repo. And it's good. It will be the second step. It will let me make commits here more consistent (not only via web interface), less mess... After that, maybe, I'll look at hggit.

...More to the point, I'm going to create a local git repo in order to merge (or not) your commits one-by-one, not all at once.

https://code-maven.com/merge-one-commit-from-a-pull-request

[keks24]

Sure thing!

It is your repository, so everything is up to you! :)

The instructions, you have found, look weird. There is actually a command for this: git cherry-pick.

https://mattstauffer.com/blog/how-to-merge-only-specific-commits-from-a-pull-request/

[halcon74]

Well, I've been a little confused working with GitHub in command line for the first time... I've got conflicts, made force-updates... The first pancake. It will get easier :)

In short, I merged include 'setuid', 'setgid' and 'sticky bit' with an additional commit of mine. That's not all, I'll continue.

I wonder that merged commits, parts of the PR, don't get marked as merged.

[halcon74]

The final result is so:

include 'setuid', 'setgid' and 'sticky bit'
75d7bda
merged, with adding my commit Add one-or-zero '?' for setuid digit

do not mix quotes, redirect errors to 'stderr' (file descriptor 2)
a9e7b8a
merged in part of redirecting errors; I like 'mixing' quotes, I used to do that for many years, it makes sense for security, not to eval more than is necessary...

check file mask at the 'check section', use verbose parameter instead…
0c76fae
merged in part of checking file mask; your spelling has an error even after correct check (negation sign '!' shouldn't be before '-z'), I corrected it

use verbose parameter
d55a386
not merged; I like set -x, set +x more, they are 'universal', I mean, they can be used with any command, including those not having a proper --verbose option

use single quotes to emphasise variables
c08cac4
not merged; the code editor that I use (SciTE), emphasizes the variables in double quotes well enough, and I think also that code behaviour (quotes effect) is anyway more important than appearance; or I didn't understand well what you meant?

correct check
9bc12ee
see above, check file mask

use a more secure regex, 'colons' and '..' would be fatal here
06e6d00
merged

Unfortunately, I had to re-commit most your commits by myself because otherwise there were conflicts due to that I merged some your commits partially (1) and to that I didn't merge some (2).

Thank you again for this PR! Can I close it now?

[halcon74]

Hmmm, I see conflicts again here, on GitHub. It's strange, because my local copy does not have any conflict, git status shows nothing special, and I just pushed to origin master.... What did go wrong here?

[keks24]

There were conflicts, due to git push --force. I resolved it on GitHub like this:

In general, I deleted my adaptions and took yours. It also works in the other direction. I actually do not know any other method to solve a merge conflict.

Before

<<<<<<< master
Exiting 1.' >&2 # or >/dev/stderr
=======
Exiting 1.' >&2
>>>>>>> master

After

Exiting 1.' >&2

Before

<<<<<<< master
		exit_err_1 "Wrong __file_owners '${__file_owners}'"
	fi
	
  if [[ ! -z "${__file_mask}" || ! "${__file_mask}" =~ ^[0124]?[0-7][0-7][0-7]$ ]]; then
		exit_err_1 "Wrong __file_mask '${__file_mask}'"
=======
		exit_err_1 'Wrong __file_owners '"${__file_owners}"
	fi
	
	if [[ -z "${__file_mask}" || ! "${__file_mask}" =~ ^[0124]?[0-7][0-7][0-7]$ ]]; then
		exit_err_1 'Wrong __file_mask '"${__file_mask}"
>>>>>>> master

After

		exit_err_1 'Wrong __file_owners '"${__file_owners}"
	fi
	
	if [[ -z "${__file_mask}" || ! "${__file_mask}" =~ ^[0124]?[0-7][0-7][0-7]$ ]]; then
		exit_err_1 'Wrong __file_mask '"${__file_mask}"

[keks24]

Oh, I forgot to submit these two reviews, when I was creating the pull-request in the night. :D Take a look. I guess, you can also resolve them.

[keks24]

not merged; the code editor that I use (SciTE), emphasizes the variables in double quotes well enough, and I think also that code behaviour (quotes effect) is anyway more important than appearance; or I didn't understand well what you meant?

I meant, you could emphasise the variable in quotes like this:

$ some_variable="/tmp/something"
$ echo "Something happened at '${some_variable}'"
Something happened at '/tmp/something'

/tmp/something is displayed in single quotes, so you can immediately tell, that a variable was used there. This is just my personal flavour I really like to do and it is nice for debugging.

[halcon74]

/tmp/something is displayed in single quotes, so you can immediately tell, that a variable was used there. This is just my personal flavour I really like to do and it is nice for debugging.

Aaah, I got it finally, thanks :) I will think about it, it's interesting.

[keks24]

Alright! I guess, we can close this now. :)