Skip to content

Conversation

GuillaumeGomez
Copy link
Member

@shepmaster shepmaster added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 7, 2018
@shepmaster
Copy link
Member

r? @QuietMisdreavus

@novacrazy
Copy link

Would it not be better to add some kind of generic syntax alias system?

For example, while this may fix searching for !, many other things like ? and any operators (+, %, /, *, [/], etc.) still give weird results.

@GuillaumeGomez
Copy link
Member Author

This is an excellent point! I'll update this PR with these new adds.

@novacrazy
Copy link

novacrazy commented Apr 8, 2018

Maybe something like this?

query = (function() {
    switch(raw) {
        case '!': return "Never";
        case '?': return "try!";
        case '+': return "Add";
        case '-': return "Sub";
        case '%': return "Rem";
        case '[':
        case ']': return "Index";
        // and so forth
        default: return raw;
    }
})();

@kennytm
Copy link
Member

kennytm commented Apr 8, 2018

[] can also refer to slices, and - could also mean the Neg trait. Could query support unioning results? 🤔

@GuillaumeGomez
Copy link
Member Author

I think I'll need to introduce a new syntax. Something like 'vec|Neg' and then it'll look for the both. What do you think of it?

@novacrazy
Copy link

Perhaps a comma would work better? As if it’s a list of search terms rather than a full expression. That would leave | free for aliasing to BitOr as well.

@kennytm
Copy link
Member

kennytm commented Apr 8, 2018

💡 Alternative: Allow items to register symbols as an alias index, like how the primitives are presented.

Rationale: Non-std docs don't care about ! being the never type. Modifying the *.js infects those non-std docs.

#[doc(primitive = "never")]
#[doc(alias = "!")]
/// blah
mod prim_never { }

#[doc(primitive = "slice")]
#[doc(alias = "[]")]
mod prim_slice { }

#[doc(alias = "-")]
pub trait Neg { }

#[doc(alias = "-")]
pub trait Sub { }

@novacrazy
Copy link

novacrazy commented Apr 8, 2018

Is there a way to make a doc attribute like that require an unstable feature flag? That’s not really my area.

I feel like it could be abused in crates, yet it should be allowed in crates on the rare occasion users need to define their own lang items, like with no_core bare-metal projects.

Edit: Can no_core crates even build documentation?

@kennytm
Copy link
Member

kennytm commented Apr 8, 2018

@novacrazy

Is there a way to make a doc attribute like that require an unstable feature flag?

Yes, that's how #[doc(cfg/masked/spotlight)]'s feature-gating works.

@GuillaumeGomez
Copy link
Member Author

@kennytm: This is a brilliant idea. I'm just a bit afraid that people might misuse it though or that readers won't be aware of it, and therefore not use it... But this remains an excellent idea. I think I'll start implementing it.

@shepmaster
Copy link
Member

that people might misuse it though

You could put restrictions that the "alias" has to be non-word characters to start with. This might mitigate overall abuse.

@GuillaumeGomez
Copy link
Member Author

Indeed!

@QuietMisdreavus
Copy link
Contributor

+1 for adding a general mechanism. Adding a feature gate to a doc-attribute parameter is easy. The compiler already whitelists the doc attribute as a whole, so adding a check for a specific parameter just involves knowing the right place to put it.

Edit: Can no_core crates even build documentation?

@novacrazy Yes - as long as the compiler can build it, so can rustdoc. For example, libcore is a #![no_core] crate, and its docs build fine.

@pietroalbini pietroalbini added T-dev-tools-rustdoc S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 16, 2018
@pietroalbini
Copy link
Member

Blocked on #49966

@kennytm
Copy link
Member

kennytm commented Apr 16, 2018

@pietroalbini My alternative in #49757 (comment) wouldn't need #49966, though if the alternative is chosen, this PR should be closed or entirely rewritten.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 17, 2018
@bors
Copy link
Collaborator

bors commented Apr 17, 2018

☔ The latest upstream changes (presumably #50033) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member Author

@kennytm: Your doc alias can be the same for multiple types so it still strongly requires the multi-query feature.

@kennytm
Copy link
Member

kennytm commented Apr 18, 2018

@GuillaumeGomez I don't think so, I expect there would be multiple dedicated entries named [] or - etc, and the search UI would be like:

screenshot_2018-04-18 23 57 50_jlnaq0

@GuillaumeGomez
Copy link
Member Author

Ok, here's all the implemented functionality. I finally went for last @kennytm's comment: no need for multiquery, I just add types at the beginning of the results and we're good!

cc @QuietMisdreavus

@rust-highfive

This comment has been minimized.

// `Never` primitive type.
if (raw === '!') {
query = 'Never';
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could delete this code now right? 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh damn! Completely forgot about it 😆

@GuillaumeGomez
Copy link
Member Author

Updated.

@QuietMisdreavus
Copy link
Contributor

Looks good! One round of requests from me:

  • Can you open up a tracking issue for the new doc_alias feature and reference it in your feature gate?
  • Could you write up some docs in src/doc/unstable-book/src/language-features/doc-alias.md talking about what the attribute does?
    • Optional: add a note in src/doc/rustdoc/src/unstable-features.md alongside the other doc attribute stuff linking to the Unstable Book page

@QuietMisdreavus
Copy link
Contributor

Great, thanks so much! r=me pending travis.

@rust-highfive

This comment has been minimized.

@kennytm
Copy link
Member

kennytm commented Apr 22, 2018

@bors r=QuietMisdreavus

@bors
Copy link
Collaborator

bors commented Apr 22, 2018

📌 Commit 1ed3e77 has been approved by QuietMisdreavus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2018
@bors
Copy link
Collaborator

bors commented Apr 22, 2018

⌛ Testing commit 1ed3e77 with merge bbdd1cf...

bors added a commit that referenced this pull request Apr 22, 2018
@bors
Copy link
Collaborator

bors commented Apr 22, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing bbdd1cf to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants