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

ignored vs ignore #510

Open
max-sixty opened this issue Aug 24, 2023 · 6 comments
Open

ignored vs ignore #510

max-sixty opened this issue Aug 24, 2023 · 6 comments

Comments

@max-sixty
Copy link
Sponsor

max-sixty commented Aug 24, 2023

(V small, as I mentioned #498, writing it because it sounds like this is last chance for crate-wide changes. Feel free to close without comment)

Why is .ignored not .ignore? We have .ignore_then & .then_ignore, but then .ignored?

Currently we seem to use the indicative mood to describe the structure of the language we're parsing — delimited_by / separated_by / repeated. And then the imperative mood to tell the parser what to do — .map / select. Consistent with that — we'd have .ignore (and maybe .label...).

Given that the rustc errors are difficult to read, to the extent we can use mood or tense to make functions clearer, that would be excellent.

(though I realize that #498 suggested going the other way — .collect would be the imperative mood, while .collected would be the indicative mood...)

@zesterer
Copy link
Owner

Currently we seem to use the indicative mood to describe the structure of the language we're parsing and then the indicative mood to tell the parser what to do

Yep, this is pretty much the rule I've been trying to keep to in my head (subconsciously, perhaps). I think it's a useful thing to delineate that which the parser cannot control (the syntax of the grammar it must pass) vs what it can control (the output that it produces).

Being explicit about this somewhere in the docs might be useful, I think.

You're right that several combinators currently break this rule:

  • ignored (should be ignore)
  • boxed (although box is a Rust keyword and boxed has prior work all over the Rust ecosystem).
  • filter (should be filtered)
  • foldl/r (should be foldedl/r)
  • labelled (should be label)
  • rewind (this is a weird one: should be rewound, but it's not really a description of the grammar so gets a pass, IMO)
  • validate (should be validated)
  • unwrapped

Special mention regarding ignore_then/then_ignore: in both cases, the 'ignore' refers to one of the two patterns. Switching tense would more correctly result in ignored_then/then_ignoring which feels even more awkward, I think, especially for people with a less intimate relationship with English.

I am happy to change these, although I think a few cases could become confusing (unwrapped in particular: .unwrap() is a common red flag in Rust code and a method on Parser of the same name might be a source of confusion about how and when a panic might occur).

Does anybody else that feels they have a stake in the library's development have thoughts/preferences about this?

@max-sixty
Copy link
Sponsor Author

Great, thanks a lot for the clear post @zesterer .

(I also realize a couple of my examples weren't correct, so this was informative for me)

@CraftSpider
Copy link
Collaborator

CraftSpider commented Aug 29, 2023

I personally really dislike foldedl and foldedr, they don't sound like anything I would ever say out-loud. The others I think would be fine changes to make.

@calder
Copy link
Contributor

calder commented Nov 19, 2023

Same, I think I would lean towards being concise and matching existing Rust names when possible over mood-correctness. So:

  • boxed because box is a Rust keyword and boxed has prior art
  • filter because it's more concise and has prior art
  • foldl because it's more concise and has prior art
  • ignore because it's more concise
  • label because it's more concise
  • rewind because it's more concise
  • validate because it's more concise
  • unwrap because it's more concise and has prior art

Even as a native English speaker the mood distinction was lost on me, and left me guessing which tense to use each time.

@nouritsu
Copy link

can I work on this, once a final decision has been made?

@zesterer
Copy link
Owner

Sure. I don't think there are really actionable conclusions though, and any work would more than likely just be a careful find + replace operation.

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

5 participants