kcoyner / rpl

intelligent recursive search/replace utility
GNU General Public License v2.0
14 stars 13 forks source link

Bless @rrthomas' fork? #7

Open encukou opened 3 years ago

encukou commented 3 years ago

Hello, @rrthomas has been maintaining a friendly fork of rpl over at https://github.com/rrthomas/rpl. That is also the version included in Debian. Would you allow @rrthomas to make that fork official – e.g. by making them a collaborator here, or even transferring the repo?

rrthomas commented 3 years ago

Hi @kcoyner, I wrote to you in July 2018 about maintaining rpl, which I have been doing since then; Debian switched to my upstream shortly afterwards.

homeworkprod commented 3 years ago

A new year has begun and I've returned to rpl to update the copyright lines in my projects :)

@rrthomas, great to see you continuing with the project!

Didn't know that Debian switched to your fork (I just followed the "homepage" link of Debian Buster's version of rpl, which still points to this repository). I'm happy that I could help with my changes :)

Since I'm interested in rpl being maintained, I support the suggestion of making @rrthomas's fork somehow official, especially if that is the only practical option there is for the time being.

In any case, thanks to you @kcoyner for your work as well!

rrthomas commented 3 years ago

Just to say, I'd be quite happy to become the "official" maintainer, whatever that means! I use rpl regularly, and have been helping packagers adopt my version. I don't have any major plans for changes/new features (which at this point will probably be a feature, not a bug, to most users!).

SkycoinSynth commented 3 years ago

RPL is find and replace.

@rrthomas You removed the -R flag from find and replace. Which is primary usage of RPL in software development.

Your justification for removing the -R flag was that it was "dangerous".

https://github.com/kcoyner/rpl/commit/753e84b769a00a4fc1caf35a31caeadf08f425ed

Maybe you should become the maintainer of "rm" and remove "rm -R" because its "dangerous"?

Your change to rpl, broke a unix/linux command that has been used and taught in schools and tutorials the same way for 40 years.

@encukou The bug tracker is full of issues because of rpl -R option was removed. This is a huge headache because xargs have problems with filenames with special characters and spaces in it.

rrthomas commented 3 years ago

@SkycoinSynth, since version 1.8, you can use recursive glob patterns to do recursive replacement safely.

I'm happy to look at issues. The rpl bug tracker is not full of issues (it's empty), and there are no issues currently open against the Debian package, so I'm not sure what issues you're referring to; could you help?

I've explained the use of xargs (which is easy!) in issue #8, but in any case, it's no longer needed.

SkycoinSynth commented 3 years ago

One of these two commands, is cleaner, simpler and more readable than the other.

rpl -R "old" "new" .
find . | xargs rpl "old" "new"

There is no possible justification for forcing people to use xargs, over a simple one letter command line argument, except out of malicious intent.

If you were the the maintainer of grep and rm, would you take it upon yourself to remove the -R flag, to force people to use xargs?

rrthomas commented 3 years ago

As documented in the man page: rpl old new '**'

SkycoinSynth commented 3 years ago

BTW. Today, I had a script that ran for a decade and which has not been edited for eight years.

And that script worked perfectly on BSD and on OSX and on my development machine. But the script failed on a docker deployment using a newer Debian image, because of your removal of this simple and essential command line argument.

In fact only ten lines of code.

And the xargs solution is much less reliable because it requires special care and extreme skill to deal with filenames with white spaces and escaped characters.

In general, I support maintaining comparability between linux distributions. And I support maintaining agreement of utilities with their documentation and the linux man pages.

rrthomas commented 3 years ago

If you find that rpl does not behave as documented, please file an issue. The man page for rpl is generated directly from the source code, so it should be correct.

SkycoinSynth commented 3 years ago

As documented in the man page: rpl old new '**'

grep -n "a" '' grep: : No such file or directory

I would in general, maintain the same standard command line arguments as supposed by find, grep, rm and other linux tools. Instead of inventing new conventions.

And I appreciate your glob improvement and unicode support is better now, but its unnecessary and undesirable to remove standard command line arguments.

SkycoinSynth commented 3 years ago

If you find that rpl does not behave as documented, please file an issue. The man page for rpl is generated directly from the source code, so it should be correct.

An internet troll has asked me "ask him to provide you the correct xargs command for filenames that may have spaces or newlines in them".

Can you answer his challenge?

rrthomas commented 3 years ago

I would in general, maintain the same standard command line arguments as supposed by find, grep, rm and other linux tools. Instead of inventing new conventions.

It's not a new convention, recursive globbing has been in shells (and Python's standard library) for decades.

rrthomas commented 3 years ago

An internet troll has asked me "ask him to provide you the correct xargs command for filenames that may have spaces or newlines in them".

Can you answer his challenge?

As I have said, rpl does not need to use find for recursive operation. I gave the find invocation to handle filenames with arbitrary characters in https://github.com/rrthomas/rpl/issues/9#issuecomment-785673782

SkycoinSynth commented 3 years ago

I would in general, maintain the same standard command line arguments as supposed by find, grep, rm and other linux tools. Instead of inventing new conventions.

It's not a new convention, recursive globbing has been in shells (and Python's standard library) for decades.

Just because globbing is a valid method, does not mean that we should handicap people by forcing them to glob.

Should we force people to glob when using grep? Should we remove the -R flag from grep?

This is a corporate issue. Maintainability issue. Standards issue.

No one in the linux community supports the removal of the -R flag. No one benefits from it. Everyone suffers.

This is a very bad attitude for a maintainer, over ten lines of code in a commonly used command line utility. I dont know if you understand how many bugs or tickets were entered over this issue already.

rrthomas commented 3 years ago

@SkycoinSynth I think I've already addressed all your points; sorry, I don't have more time for this now.

SkycoinSynth commented 3 years ago

@SkycoinSynth I think I've already addressed all your points; sorry, I don't have more time for this now.

You made false claims. You claimed that your example of shell commands, that replace rpl -R can be used in the same circumstances and when tested, the commands you provided (there were three of them) do no all handle unicode file names, file names with spaces and file names with new line characters in them, in the correct way.


I do not believe you have given sufficient justification for neutering the rpl command and your justifications show that you are not suitable as a maintainer for Debian.

rrthomas commented 3 years ago

You made false claims. You claimed that your example of shell commands, that replace rpl -R can be used in the same circumstances and when tested, the commands you provided (there were three of them) do no all handle unicode file names, file names with spaces and file names with new line characters in them, in the correct way.

Here are the three commands I gave in https://github.com/rrthomas/rpl/issues/9#issuecomment-785673782 :

find . | xargs rpl OLD NEW
find . -exec rpl OLD NEW {} +
rpl OLD NEW '**/*'

Note, I said about the first: "this will work for all filenames that don't contain a newline."

Here's a shell session:

$ rpl --version
rpl 1.9rc3
…
$ echo cat > 'une fiche
démo'
$ ls
'une fiche'$'\n''démo'
$ find . -exec rpl cat dog {} +

rpl: Replacing "cat" with "dog" (case sensitive; partial words matched)

rpl: 1 matches replaced in 1 file
$ cat 'une fiche
démo' 
dog
$ rpl dog fish '**'

rpl: Replacing "dog" with "fish" (case sensitive; partial words matched)

rpl: 1 matches replaced in 1 file
$ cat 'une fiche
démo' 
fish

In other words, the two methods I claimed work with filenames with any character in them do indeed work for a file whose name contains a space, a newline, and a non-ASCII character.