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

Command to do individual interactive-sr's #266

Open
vr8hub opened this issue Mar 15, 2020 · 6 comments
Open

Command to do individual interactive-sr's #266

vr8hub opened this issue Mar 15, 2020 · 6 comments

Comments

@vr8hub
Copy link
Contributor

vr8hub commented Mar 15, 2020

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

vr8hub commented Mar 15, 2020

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

vr8hub commented Mar 15, 2020

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

@acabal
Copy link
Member

acabal commented Mar 15, 2020

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

acabal commented Mar 15, 2020

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.

@robinwhittleton
Copy link
Member

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?

@acabal
Copy link
Member

acabal commented Mar 15, 2020

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.

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

3 participants