kadircet added a comment. > I see little value in not checking FallbackStyle (even if it's not used).
note that this patch doesn't disable fallbackstyle checking, it is still checked, but not eagerly. > However, I do see value in an early warning (error) on an incorrect style > that can pop up later if not caught soon enough. i don't follow the value proposition here. if user has a valid way to provide "style", the fallbackstyle will never be used (malformed or otherwise). so in such a case, user is currently going to get a "hard error" preventing them from formatting with the style they provided whenever they have a malformed fallback-style. what i am trying to say is, "earliness" of the warning/error in such a scenario doesn't seem to benefit any user (at least to me). since fixing their fallbackstyle won't change anything for formatting of the particular file in question, and they'll still see the error whenever they try to format a file that needs the fallbackstyle. hence currently the eager validation of a malformed FallbackStyle is *only* preventing a user with a valid Style from formatting their file (it is erroring in all other cases, but there's nothing much to be done for those). to add, there will never be a file formatted with the incorrect style. > I return the question, are there any workflows that would benefit from the > proposed behaviour? Apart from those that have already a faulty config? that's the exact set of workflows that this change is going to effect(if `FallbackStyle` is valid, this patch is a no-op). so, no, i don't think there will be any other workflows that can benefit from this change, but it feels like this is a non-empty set of users (while not regressing any users that benefit from current error/warning, as they'll still be surfaced when needed). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95538/new/ https://reviews.llvm.org/D95538 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
