search

LibraryCarpentry / lc-data-intro

Library Carpentry: Introduction to Working with Data (Regular Expressions)
https://librarycarpentry.org/lc-data-intro/
Other
29 stars 84 forks source link

02-match-extract-strings: Fix solution and typos in Finish the expression #196

Closed ldko closed 1 year ago

ldko commented 2 years ago

This PR is to fix the solution and typos in the Finish the expression challenge. At some point the last + was removed from the solution. Also, there is no need to put the \w after the literal . in square brackets since it is only one option.

As a side note, in challenges where you give hints, you could do them as dropdowns to give people a chance to solve the challenge without them e.g.:

> > ## Hint
> > The `.` is also a metacharacter, so you will have to use the escape `\`
> > to express a literal period.

hint_dropdown

I did not include the Hint as a dropdown in this PR, since that is not how you are doing hints generally.

LRDamsma commented 1 year ago

This is almost a year old now. Who can fix this?

ostephens commented 1 year ago

Generally I'd hope one of the maintainers could look at it @sharilaster @ppival @antonangelo @ccronje If they aren't able to then I'm able to merge (none of the changes here look likely to be a problem to me) it but generally I'd hope a maintainer could at least take a look before that happens

sharilaster commented 1 year ago

Hi @LRDamsma and @ostephens, sorry I overlooked this request when it first came in. I think the suggestion from @ldko to improve hints should be submitted as an issue; if others concur, then once that is done I can merge and close this PR.

ostephens commented 1 year ago

Thanks @sharilaster.

I think the suggestion from @ldko to improve hints should be submitted as an issue; if others concur, then once that is done I can merge and close this PR.

I agree that needs to be a separate issue, but I don't think that needs to hold up merging and closing this PR? (as this PR uses the current approach to hints)

sharilaster commented 1 year ago

I agree that needs to be a separate issue, but I don't think that needs to hold up merging and closing this PR? (as this PR uses the current approach to hints)

@ostephens I just want to make sure it isn't lost as a suggestion. I am not sure if other maintainers are still monitoring this lesson, but I can wait a day or so to see if others weigh in, and then merge/close at that point. Would that work?

ostephens commented 1 year ago

Sure! I've created an issue for that now #200

sharilaster commented 1 year ago

That looks great, thank you so much @ostephens! And thank you @ldko for the suggestion and @LRDamsma for the nudge to address this PR.