Clean up public interfaces to add name to unnamed bool |
||||
Issue description
Example from ./third_party/WebKit/Source/platform/fonts/FontDataCache.h
scoped_refptr<SimpleFontData> Get(const FontPlatformData*,
ShouldRetain = kRetain,
bool = false);
Expected to have:
scoped_refptr<SimpleFontData> Get(const FontPlatformData*,
ShouldRetain = kRetain,
bool subpixel_ascent_descent = false);
Other potential 19 occurrences:
$ git ls-files | grep -E '*\.(h)$' | xargs egrep --color "bool = (false|true)"
third_party/WebKit/Source/core/dom/ContainerNode.h: void SetActive(bool = true) override;
third_party/WebKit/Source/core/dom/ContainerNode.h: void SetHovered(bool = true) override;
third_party/WebKit/Source/core/frame/LocalFrame.h: void SetInViewSourceMode(bool = true);
third_party/WebKit/Source/core/html/HTMLAnchorElement.h: void SetActive(bool = true) final;
third_party/WebKit/Source/core/html/ImageData.h: ImageData* CropRect(const IntRect&, bool = false);
third_party/WebKit/Source/core/html/forms/HTMLFormControlElement.h: void SetAutofilled(bool = true);
third_party/WebKit/Source/core/html/forms/HTMLLabelElement.h: void SetActive(bool = true) override;
third_party/WebKit/Source/core/html/forms/HTMLLabelElement.h: void SetHovered(bool = true) override;
third_party/WebKit/Source/core/html/forms/SpinButtonElement.h: void SetHovered(bool = true) override;
third_party/WebKit/Source/core/layout/LayoutBR.h: bool = false /* firstLine */,
third_party/WebKit/Source/core/layout/LayoutBlock.h: virtual void ComputeOverflow(LayoutUnit old_client_after_edge, bool = false);
third_party/WebKit/Source/core/layout/LayoutBlockFlow.h: void SetMustDiscardMarginBefore(bool = true);
third_party/WebKit/Source/core/layout/LayoutBlockFlow.h: void SetMustDiscardMarginAfter(bool = true);
third_party/WebKit/Source/core/layout/svg/line/SVGInlineTextBox.h: bool = true) const override;
third_party/WebKit/Source/platform/ContextMenuItem.h: void SetChecked(bool = true);
third_party/WebKit/Source/platform/ContextMenuItem.h: void SetEnabled(bool = true);
third_party/WebKit/Source/platform/fonts/FontCache.h: bool = false);
third_party/WebKit/Source/platform/fonts/FontDataCache.h: bool = false);
third_party/WebKit/Source/platform/transforms/IdentityTransformOperation.h: bool = false) override {
,
Nov 5 2017
,
Nov 9 2017
,
Nov 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b8c5f68c4c9408046679cd0f03497aae5286c96 commit 5b8c5f68c4c9408046679cd0f03497aae5286c96 Author: Luciano Pacheco <lucmult@chromium.org> Date: Fri Nov 10 05:38:09 2017 Add explicit argument names to improve readability Add named arguments to "bool" arguments with default values for readability, as identified by the following command: $ git ls-files | grep -E '*\.(h)$' | xargs egrep --color \ "bool = (false|true)" Also update the unnamed double argument in the transform include file, IdentityTransformOperation.h Bug: 781101 Change-Id: I7cd541b1329063805c60b3ee86247d494f8793b4 Reviewed-on: https://chromium-review.googlesource.com/759272 Reviewed-by: Noel Gordon <noel@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Chris Watkins <watk@chromium.org> Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#515457} [modify] https://crrev.com/5b8c5f68c4c9408046679cd0f03497aae5286c96/third_party/WebKit/Source/core/html/ImageData.h [modify] https://crrev.com/5b8c5f68c4c9408046679cd0f03497aae5286c96/third_party/WebKit/Source/core/layout/LayoutBlock.h [modify] https://crrev.com/5b8c5f68c4c9408046679cd0f03497aae5286c96/third_party/WebKit/Source/core/layout/svg/line/SVGInlineTextBox.h [modify] https://crrev.com/5b8c5f68c4c9408046679cd0f03497aae5286c96/third_party/WebKit/Source/platform/fonts/FontCache.h [modify] https://crrev.com/5b8c5f68c4c9408046679cd0f03497aae5286c96/third_party/WebKit/Source/platform/fonts/FontDataCache.h [modify] https://crrev.com/5b8c5f68c4c9408046679cd0f03497aae5286c96/third_party/WebKit/Source/platform/transforms/IdentityTransformOperation.h
,
Dec 3 2017
Hi all, This bug was still open because kouhei@ suggested on review to apply ENUM to some of those bools as per our style guide [1] I did quick review of booleans on Blink and couldn't find any that was terribly bad as bool and with the right argument name. I used the following command to retrieve ~834 bool definitions (filtered out some of the probably well named args see regex on grep -v): $ git ls-files |egrep "(third_party/WebKit/Source/|third_party/WebKit/common|third_party/WebKit/public/)" | grep -E '*\.(h)$' | xargs ack-grep "bool \w+ =?" |egrep -v "(is_|has_|[Hh]as[A-Z]|did_|does_|had_|was_|static const bool|static constexpr)" |egrep "bool (\w+)" With that said, I consider this bug finished. kouhei@ if you still think we need further work on this area, I suggest to open a new bug. [1] - https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/blink-c++.md#Prefer-enums-to-bools-for-function-parameters REPLY |
||||
►
Sign in to add a comment |
||||
Comment 1 by lucmult@chromium.org
, Nov 3 2017