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

Bless @rrthomas' fork? #7

Open
encukou opened this issue Oct 5, 2020 · 17 comments
Open

Bless @rrthomas' fork? #7

encukou opened this issue Oct 5, 2020 · 17 comments

Comments

@encukou
Copy link

encukou commented Oct 5, 2020

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
Copy link

rrthomas commented Oct 5, 2020

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
Copy link

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
Copy link

rrthomas commented Jan 1, 2021

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
Copy link

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".

753e84b

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
Copy link

@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
Copy link

SkycoinSynth commented Feb 25, 2021

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
Copy link

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

@SkycoinSynth
Copy link

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
Copy link

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
Copy link

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
Copy link

SkycoinSynth commented Feb 25, 2021

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
Copy link

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
Copy link

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 rrthomas#9 (comment)

@SkycoinSynth
Copy link

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
Copy link

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

@SkycoinSynth
Copy link

SkycoinSynth commented Mar 2, 2021

@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
Copy link

rrthomas commented Mar 2, 2021

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 rrthomas#9 (comment) :

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.

teto pushed a commit to NixOS/nixpkgs that referenced this issue Oct 2, 2021
* rpl: update to 1.10
Changes upstream as discussed in kcoyner/rpl#7
- manpage missing because of a missing python package.
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

No branches or pull requests

4 participants