dgoldman added inline comments.
================
Comment at: clang-tools-extra/clangd/TextMarks.h:22
+/// Represents a programmer specified mark/note, typically used to easily
locate
+/// or differentiate code. e.g. a `pragma mark`, a `TODO`.
+///
----------------
sammccall wrote:
> dgoldman wrote:
> > sammccall wrote:
> > > This seems like premature abstraction - you don't handle TODOs here, nor
> > > are they appropriate for the single use case we have. Moreover they don't
> > > satisfy the definition here: `int foo; // TODO: turn into a float` the
> > > TODO doesn't occupy the whole line.
> > >
> > > If we need it, the appropriate generalization seems just as likely to be
> > > "pragmas" or "directives" as "textual marks".
> > >
> > > Similarly, it's not clear that the logic around interpreting the strings
> > > e.g. as groups needs to be shared across features, nor is it terribly
> > > complicated.
> > > So the interface here seems like it could be much simpler:
> > > ```
> > > struct PragmaMark {
> > > unsigned Line;
> > > string Text;
> > > }
> > > unique_ptr<PPCallbacks> collectPragmaMarksCallback(const SourceManager&,
> > > vector<PragmaMark> &Out);
> > > ```
> > >
> > > I'd suggest putting this in CollectMacros.h (which would be misnamed but
> > > not terribly, or we could always rename) rather than adding a new header.
> > We don't handle them at the moment, but Xcode does (for both Swift + ObjC):
> > https://medium.com/@cboynton/todo-make-your-notes-on-xcode-stand-out-5f5124ec064c.
> > You're right that they don't necessarily have to occupy the whole line
> > though, it's possible to have multiple per line although I'm not sure how
> > often that is used in practice.
> This seems like a very different case, and I'm not convinced we should
> support it.
>
> That `#pragma mark`s are used for grouping seems like the strongest argument
> for including them in the outline. TODOs are not used for grouping.
>
> The majority of `#pragma mark`s are written with XCode's conventions in mind,
> the majority of TODO comments are not. So there's a real question of whether
> authors want this. (And whether it should include all comments, or other
> kinds of structured comments...)
>
> I'd suggest keeping the scope small and concrete. If you'd like to add
> abstractions because more cases are imminent, we probably need to get
> consensus on (some of) these cases first.
>
>
Technically Xcode also supports `// MARK` comments as well, but almost all
users internally use `#pragma mark`. I do think the TODO/FIXME in the outline
could be useful (Xcode does it...), but if you're against it I'll remove this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105904/new/
https://reviews.llvm.org/D105904
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
