tools icon indicating copy to clipboard operation
tools copied to clipboard

Command to do individual interactive-sr's

Open vr8hub opened this issue 4 years ago • 6 comments

You're right that doing all of the interactive-sr's at once is probably a bad idea. But providing a command that does the grunt work of the individual commands, so a producer doesn't have to remember or cut-and-paste the commands from the website, might still be beneficial.

To that end, here is the join-words command. Since it's a standalone command file, and since I don't know if you want it, I'm just attaching it instead of opening a PR. (I had to name it .txt to get GitHub to take it.)

Essentially, instead of running these two commands as listed in the Step-By-Step guide: se interactive-sr "/\v([Ss])ome one/\1omeone/" src/epub/text/* and then git commit -am "[Editorial] some one -> someone" you would instead just run this command: se join-words someone and it would do both of them.

There is a --nocommit option if you don't want to automatically commit. I made committing the default because 1) that's how I use it, and 2) I think it's what is wanted 99% of the time; the other 1% it's easy enough to undo the commit. I'd rather have to undo a commit 1 out of 100 then have to specify a --commit flag 99 out of 100.

If you don't think this is a good idea, either, just close the issue and nothing further needs to be said. :) If you do think it's a good idea, but don't like something in the code, just let me know and I'll fix it.

I was just going to call the interactive-sr command from this one, but I couldn't figure out how to call one command from another (I tried two or three things and couldn't get them to work). Since the actual work in interactive-sr is done in a single line, I just copied that line, using the variable names from this command. I was using another subprocess call to do the commit, too, until I happened across your use of the git module in one of the other commands. Nice! join_words.txt

vr8hub avatar Mar 15 '20 00:03 vr8hub

Grrr. Sorry, I realized literally five seconds after I hit send that I hadn't run pylint. As a result, I changed subprocess.run back to subprocess.call, but I see that there are other instances (at least in 1.2.3, I haven't updated since then) of run, and pylint's throwing the "should have a check" message there as well. I don't have time right this second to go investigate, so I just changed it back to call to match interactive-sr. join_words.txt

vr8hub avatar Mar 15 '20 00:03 vr8hub

Sigh. After the anyway regex conversation, I thought I had looked at the other regex's and seen they were all the same. But no, a couple of them have beginning-of-word markers on them, which I just discovered when found "many things" when running this for anything. And that led me to a couple of others. I have now done what I should have done in the first place, which is copy each and every regex from the website to here. My apologies. join_words.txt

vr8hub avatar Mar 15 '20 04:03 vr8hub

I'm interested in the idea but I'd like to brainstorm a little more on what form it could take.

I don't know if we need a separate command for this, because what we're doing is really modernizing spelling. So if we're going to script this, why not include it as part of the modernize-spelling command. If we're doing that, then I'm OK with dropping the individual commit messages and just calling the whole thing "modernizing spelling".

If we do that, we could add a --skip-interactive option to skip these changes.

I'm also concerned that if we lump all these together into one sequence, it may not be clear to the user what replacement is going to happen next. So before each sequence, we can print something like "Next: any way -> anyway, press enter to continue" so that the user knows what the next replacement set is going to be.

@drgrigg, @robinwhittleton any input?

acabal avatar Mar 15 '20 19:03 acabal

To clarify a little, I'm envisioning something a bit different than the presentation in this issue. The command in this issue is kind of a shortcut for each individual replacement, which is OK but not a big difference between cutting and pasting each incantation. (Nobody is going to remember to run a shortcut command for each individual replacement, so they're going to copy and paste from the guide anyway.)

Scripting makes sense in the scenario where we automate the whole lot. But, in a long-running interactive scenario, we have to put enough breakpoints in for the user to be able to stop the process sanely, to understand what's coming next, etc.

acabal avatar Mar 15 '20 20:03 acabal

Not a lot of input from me, but I guess we’d add these to the existing messages (e.g. for staid). Is there a danger that will get too noisy? Should we add a dry run switch that prints the messages but doesn’t make any updates?

robinwhittleton avatar Mar 15 '20 20:03 robinwhittleton

The messages you're talking about are already only warning messages, they don't make any changes. They just alert the producer that a tricky word was found and that manual review is required.

acabal avatar Mar 15 '20 20:03 acabal