[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

Also, don't we want to change the title and summary of the commit/revision? 
Because it does not reflect the real changes now in the repo


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

Thanks for the merge, I now understand more how this all works and what we want 
from these scripts.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e8fff26f374: [clang-format][docs] Fix documentation of 
clang-format BasedOnStyle type (authored by FederAndInk, committed by 
MyDeveloperDay).

Changed prior to commit:
  https://reviews.llvm.org/D108765?vs=370953=374739#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt

Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,78 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
-import collections
+import inspect
 import os
 import re
+from typing import Set
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURALS_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+
+plurals: Set[str] = set()
+with open(PLURALS_FILE, 'a+') as f:
+  f.seek(0)
+  plurals = set(f.read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+if not hasattr(register_plural, "generated_new_plural"):
+  print('Plural generation: you can use '
+  f'`git checkout -- {os.path.relpath(PLURALS_FILE)}` '
+  'to reemit warnings or `git add` to include new plurals\n')
+register_plural.generated_new_plural = True
+
+plurals.add(plural)
+with open(PLURALS_FILE, 'a') as f:
+  f.write(plural + '\n')
+cf = inspect.currentframe()
+lineno = ''
+if cf and cf.f_back:
+  lineno = ':' + str(cf.f_back.f_lineno)
+print(f'{__file__}{lineno} check if plural of {singular} is {plural}', file=os.sys.stderr)
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+
+  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +95,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +140,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
@@ -129,7 +184,7 @@
 
 def read_options(header):
   class State(object):
-BeforeStruct, Finished, InStruct, InNestedStruct, InNestedFieldComent, \
+BeforeStruct, Finished, InStruct, InNestedStruct, InNestedFieldComment, \
 InFieldComment, InEnum, InEnumMemberComment = range(8)
   state = State.BeforeStruct
 
@@ -173,12 +228,12 @@
 raise Exception('Invalid 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

@MyDeveloperDay is it all good? I'm sorry for all the misunderstanding,


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 370953.
FederAndInk added a comment.

only print info on generated plurals if there is a new plural


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt

Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,78 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
-import collections
+import inspect
 import os
 import re
+from typing import Set
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURALS_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+
+plurals: Set[str] = set()
+with open(PLURALS_FILE, 'a+') as f:
+  f.seek(0)
+  plurals = set(f.read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+if not hasattr(register_plural, "generated_new_plural"):
+  print('Plural generation: you can use '
+  f'`git checkout -- {os.path.relpath(PLURALS_FILE)}` '
+  'to reemit warnings or `git add` to include new plurals\n')
+register_plural.generated_new_plural = True
+
+plurals.add(plural)
+with open(PLURALS_FILE, 'a') as f:
+  f.write(plural + '\n')
+cf = inspect.currentframe()
+lineno = ''
+if cf and cf.f_back:
+  lineno = ':' + str(cf.f_back.f_lineno)
+print(f'{__file__}{lineno} check if plural of {singular} is {plural}', file=os.sys.stderr)
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+
+  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +95,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +140,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
@@ -121,7 +176,7 @@
 
 def read_options(header):
   class State(object):
-BeforeStruct, Finished, InStruct, InNestedStruct, InNestedFieldComent, \
+BeforeStruct, Finished, InStruct, InNestedStruct, InNestedFieldComment, \
 InFieldComment, InEnum, InEnumMemberComment = range(8)
   state = State.BeforeStruct
 
@@ -165,12 +220,12 @@
 raise Exception('Invalid format, expected comment, field or enum')
 elif state == State.InNestedStruct:
   if line.startswith('///'):
-state = State.InNestedFieldComent
+state = 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I still think the printing is unnecessary, tell me when the plurals will be 
different otherwise keep quiet? otherwise, it confuses people making them think 
they have to do something


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 370944.
FederAndInk added a comment.

remove one print l17, tell me if we don't want the second print (now l17)

fixed typo in the script coment -> comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt

Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,74 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
-import collections
+import inspect
 import os
 import re
+from typing import Set
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURALS_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+print('Plural generation: you can use '
+  f'`git checkout -- {os.path.relpath(PLURALS_FILE)}` '
+  'to reemit warnings or `git add` to include new plurals\n')
+plurals: Set[str] = set()
+with open(PLURALS_FILE, 'a+') as f:
+  f.seek(0)
+  plurals = set(f.read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+with open(PLURALS_FILE, 'a') as f:
+  f.write(plural + '\n')
+cf = inspect.currentframe()
+lineno = ''
+if cf and cf.f_back:
+  lineno = ':' + str(cf.f_back.f_lineno)
+print(f'{__file__}{lineno} check if plural of {singular} is {plural}', file=os.sys.stderr)
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+
+  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +91,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +136,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
@@ -121,7 +172,7 @@
 
 def read_options(header):
   class State(object):
-BeforeStruct, Finished, InStruct, InNestedStruct, InNestedFieldComent, \
+BeforeStruct, Finished, InStruct, InNestedStruct, InNestedFieldComment, \
 InFieldComment, InEnum, InEnumMemberComment = range(8)
   state = State.BeforeStruct
 
@@ -165,12 +216,12 @@
 raise Exception('Invalid format, expected comment, field or enum')
 elif state == State.InNestedStruct:
   if line.startswith('///'):
-state = State.InNestedFieldComent
+state = State.InNestedFieldComment
 comment = 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:17
+PLURALS_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+print(f'generated plurals (for yaml type) are stored in {PLURALS_FILE}')
+print('you can use `git checkout -- plurals.txt` to reemit warnings or `git 
add`\n')

MyDeveloperDay wrote:
> printing this is just unnecessary please remove
Ok, should I also remove line 18?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:17
+PLURALS_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+print(f'generated plurals (for yaml type) are stored in {PLURALS_FILE}')
+print('you can use `git checkout -- plurals.txt` to reemit warnings or `git 
add`\n')

printing this is just unnecessary please remove


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 370883.
FederAndInk added a comment.

rebase main


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt

Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,73 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
-import collections
+import inspect
 import os
 import re
+from typing import Set
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURALS_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+print(f'generated plurals (for yaml type) are stored in {PLURALS_FILE}')
+print('you can use `git checkout -- plurals.txt` to reemit warnings or `git add`\n')
+plurals: Set[str] = set()
+with open(PLURALS_FILE, 'a+') as f:
+  f.seek(0)
+  plurals = set(f.read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+with open(PLURALS_FILE, 'a') as f:
+  f.write(plural + '\n')
+cf = inspect.currentframe()
+lineno = ''
+if cf and cf.f_back:
+  lineno = ':' + str(cf.f_back.f_lineno)
+print(f'{__file__}{lineno} check if plural of {singular} is {plural}', file=os.sys.stderr)
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+
+  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +90,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +135,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** (``string``)
+**BasedOnStyle** (``String``)
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration
@@ -166,7 +166,7 @@
 
 .. START_FORMAT_STYLE_OPTIONS
 
-**AccessModifierOffset** (``int``)
+**AccessModifierOffset** (``Integer``)
   The extra indent or outdent of access modifiers, e.g. ``public:``.
 
 **AlignAfterOpenBracket** 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could you just rebase, I'm not getting a clean merge of 
ClangFormatStyleOptions.rst


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk marked 8 inline comments as done.
FederAndInk added a comment.

Ok, so I removed git invocation and I tell the user what are their options, at 
least warnings are emitted for new plurals, and they will be shown in git 
status/diff and in revisions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 370796.
FederAndInk added a comment.

remove git invocation and user input, tell the user what the plurals are used 
for


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,73 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
-import collections
+import inspect
 import os
 import re
+from typing import Set
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURALS_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+print(f'generated plurals (for yaml type) are stored in {PLURALS_FILE}')
+print('you can use `git checkout -- plurals.txt` to reemit warnings or `git add`\n')
+plurals: Set[str] = set()
+with open(PLURALS_FILE, 'a+') as f:
+  f.seek(0)
+  plurals = set(f.read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+with open(PLURALS_FILE, 'a') as f:
+  f.write(plural + '\n')
+cf = inspect.currentframe()
+lineno = ''
+if cf and cf.f_back:
+  lineno = ':' + str(cf.f_back.f_lineno)
+print(f'{__file__}{lineno} check if plural of {singular} is {plural}', file=os.sys.stderr)
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+
+  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +90,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +135,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
Index: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

For me it feels enough to output to stderr that we are missing plurals and what 
they are for,  it’s not like you can say what they should be is it? This will 
happen so infrequently that it’s not worth the environment issues that checking 
git will cause.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

I think interacting with git or even blocking for input doesn’t feel right

I have an issue out on llvm preserve checks that would run this tool and that 
couldn’t block like this


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:21-25
+  # To allow testing with an untracked PLURAL_FILE
+  open(PLURAL_FILE, 'w').close() # TODO: remove this line when review is 
accepted
+  # TODO: use check_call when review is accepted
+  subprocess.call(['git', 'checkout', '--', PLURAL_FILE])
+

Just to let you know, I've made modifications to be able to test the code with 
an untracked plurals file. We might want stricter checks by calling check_call 
instead, also we won't need line 22 above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

From my point we can try that one, if there are problems we have plenty of time 
to revert it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 370751.
FederAndInk added a comment.

use correct python assignment from tuple, ask the user if they want to invoke 
git, use call instead of check_call to allow testing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,76 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
 import collections
+import inspect
 import os
 import re
+import subprocess
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+
+clean_plurals = input('Reset plurals file (to reemit warnings)? [y/N] ')
+if clean_plurals.lower() == 'y':
+  # To allow testing with an untracked PLURAL_FILE
+  open(PLURAL_FILE, 'w').close() # TODO: remove this line when review is accepted
+  # TODO: use check_call when review is accepted
+  subprocess.call(['git', 'checkout', '--', PLURAL_FILE])
+
+plurals = set(open(PLURAL_FILE).read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+open(PLURAL_FILE, 'a').write(plural + '\n')
+cf = inspect.currentframe()
+lineno = ''
+if cf and cf.f_back:
+  lineno = ':' + str(cf.f_back.f_lineno)
+print(f'{__file__}{lineno} check if plural of {singular} is {plural}')
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+  
+  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +93,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +138,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

In D108765#2974153 , @MyDeveloperDay 
wrote:

>   error: pathspec './plurals.txt' did not match any file(s) known to git
>   Traceback (most recent call last):
> File "./dump_format_style.py", line 18, in 
>   subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
> File "/usr/lib/python3.8/subprocess.py", line 364, in check_call
>   raise CalledProcessError(retcode, cmd)
>   subprocess.CalledProcessError: Command '['git', 'checkout', '--', 
> './plurals.txt']' returned non-zero exit status 1.

This is expected if `plurals.txt` is not in git yet, there is `call` that will 
do nothing on errors, but I use `check_call` to report errors from `git 
checkout -- plurals.txt`. To test, you can either replace temporarily the 
`check_call` by a `call` in clang/docs/tools/dump_format_style.py:18 or get the 
commit/create a temporary commit with plurals.txt

Maybe to simplify the testing/review procedure, I'll change `check_call` by 
`call` and if it is accepted, change it back to the checked version.




Comment at: clang/docs/tools/dump_format_style.py:9
 import re
+import inspect
+import subprocess

MyDeveloperDay wrote:
> FederAndInk wrote:
> > HazardyKnusperkeks wrote:
> > > I think these should be sorted.
> > ok, it will be done
> are these standard imports or are we going to have to pip install something?
These are all standard imports, no worries :) 
(https://docs.python.org/3/library/subprocess.html)

otherwise, I would have directly used python inflect/or any other package to 
solve the plural problem



Comment at: clang/docs/tools/dump_format_style.py:18
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())

MyDeveloperDay wrote:
> FederAndInk wrote:
> > HazardyKnusperkeks wrote:
> > > So you would add a plurals.txt in git and make the change visible through 
> > > git diff? What about just reordering? I.e. `Strings` is on line 2, but 
> > > after a change in line 1. Maybe sort the output?
> > > 
> > > I'm not against this procedure, but also not in favor. :)
> > This line is used to restore the version of plurals.txt to HEAD, so when 
> > calling the script multiple times, it keeps showing new plurals until 
> > plurals.txt gets committed.
> > 
> > > So you would add a plurals.txt in git and make the change visible through 
> > > git diff?
> > 
> > yes, that's it
> > 
> > > What about just reordering?
> > 
> > I don't think we want ordering, it is ordered from first plural generated 
> > to last/new one, so git diff will only show new plurals
> I'm personally not in favour of this script calling back to git
Well, I was inspired by other python scripts in the llvm-project repo that use 
`subprocess` to call `git`, it just touches the plurals.txt files to allow the 
user calling the script multiple times and be warned of the new plurals each 
time until it is committed. We could do without it, but we would lose a part of 
automation and the user of the script would have to partly manage the 
plurals.txt file.

A semi-solution I see is to at least tell the user to use `git checkout -- 
plurals.txt` if they want to clean up plurals/regenerate them and see which 
ones are new and/or call `git diff -- plurals.txt` to show new plurals which 
may be less "problematic" than `git checkout -- plurals.txt`. What do you think?

Tl;dr of solutions:

1. reconsider as this technic is in use in other scripts in llvm-project
2. call `git diff` instead of `git checkout` leading to less consistent and 
precise messages
3. just tell the user what are their options (printing 'you can use `git 
checkout -- plurals.txt` or `git diff -- plurals.txt`'...)
4. other ideas?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

  error: pathspec './plurals.txt' did not match any file(s) known to git
  Traceback (most recent call last):
File "./dump_format_style.py", line 18, in 
  subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
File "/usr/lib/python3.8/subprocess.py", line 364, in check_call
  raise CalledProcessError(retcode, cmd)
  subprocess.CalledProcessError: Command '['git', 'checkout', '--', 
'./plurals.txt']' returned non-zero exit status 1.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:9
 import re
+import inspect
+import subprocess

FederAndInk wrote:
> HazardyKnusperkeks wrote:
> > I think these should be sorted.
> ok, it will be done
are these standard imports or are we going to have to pip install something?



Comment at: clang/docs/tools/dump_format_style.py:18
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())

FederAndInk wrote:
> HazardyKnusperkeks wrote:
> > So you would add a plurals.txt in git and make the change visible through 
> > git diff? What about just reordering? I.e. `Strings` is on line 2, but 
> > after a change in line 1. Maybe sort the output?
> > 
> > I'm not against this procedure, but also not in favor. :)
> This line is used to restore the version of plurals.txt to HEAD, so when 
> calling the script multiple times, it keeps showing new plurals until 
> plurals.txt gets committed.
> 
> > So you would add a plurals.txt in git and make the change visible through 
> > git diff?
> 
> yes, that's it
> 
> > What about just reordering?
> 
> I don't think we want ordering, it is ordered from first plural generated to 
> last/new one, so git diff will only show new plurals
I'm personally not in favour of this script calling back to git


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 369519.
FederAndInk marked an inline comment as done.
FederAndInk added a comment.

add common plural rules, use python3 explicitly, reorder imports


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -1,23 +1,66 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse the FormatStyle struct from Format.h and update the
 # documentation in ../ClangFormatStyleOptions.rst automatically.
 # Run from the directory in which this file is located to update the docs.
 
 import collections
+import inspect
 import os
 import re
+import subprocess
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+open(PLURAL_FILE, 'a').write(plural + '\n')
+cf = inspect.currentframe()
+print(f'{__file__}:{cf.f_back.f_lineno} check if plural of {singular} is {plural}')
+  return plural
+
+def pluralize(word: str):
+  lword = word.lower()
+  if len(lword) >= 2 and lword[-1] == 'y' and lword[-2] not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  elif lword.endswith(('s', 'sh', 'ch', 'x', 'z')):
+return register_plural(word, word[:-1] + 'es')
+  elif lword.endswith('fe'):
+return register_plural(word, word[:-2] + 'ves')
+  elif lword.endswith('f') and not lword.endswith('ff'):
+return register_plural(word, word[:-1] + 'ves')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+  
+  [subtype, napplied] = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +83,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +128,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk marked 2 inline comments as done.
FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:9
 import re
+import inspect
+import subprocess

HazardyKnusperkeks wrote:
> I think these should be sorted.
ok, it will be done



Comment at: clang/docs/tools/dump_format_style.py:18
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())

HazardyKnusperkeks wrote:
> So you would add a plurals.txt in git and make the change visible through git 
> diff? What about just reordering? I.e. `Strings` is on line 2, but after a 
> change in line 1. Maybe sort the output?
> 
> I'm not against this procedure, but also not in favor. :)
This line is used to restore the version of plurals.txt to HEAD, so when 
calling the script multiple times, it keeps showing new plurals until 
plurals.txt gets committed.

> So you would add a plurals.txt in git and make the change visible through git 
> diff?

yes, that's it

> What about just reordering?

I don't think we want ordering, it is ordered from first plural generated to 
last/new one, so git diff will only show new plurals


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk marked an inline comment as done.
FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:26
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:

HazardyKnusperkeks wrote:
> FederAndInk wrote:
> > MyDeveloperDay wrote:
> > > This failed for me with invalid syntax
> > Oh, ok, sorry, I might be using to recent python features, I'll remove type 
> > specifier, what is the recommended python version to use?
> > Oh, ok, sorry, I might be using to recent python features, I'll remove type 
> > specifier, what is the recommended python version to use?
> 
> That **I** can not answer. I run
> ```$ python --version
> Python 3.9.5
> ```
Ok thanks, this is a reasonably recent version, I think we might want to 
explicitly specify python3 in the script to avoid using python2, I'll upload 
the diffs immediately.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D108765#2972159 , @FederAndInk 
wrote:

> And again, I don't really understand if we are allowed or not to pull in a 
> dependency such as pluralizer or inflect, this would be another idea

My Python knowledge is very limited, but if it runs out of the box, or with 
very limited required user action with a reasonably new Python (if Python 2 can 
be reasonably new is another question), I think it would be fine.




Comment at: clang/docs/tools/dump_format_style.py:9
 import re
+import inspect
+import subprocess

I think these should be sorted.



Comment at: clang/docs/tools/dump_format_style.py:18
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())

So you would add a plurals.txt in git and make the change visible through git 
diff? What about just reordering? I.e. `Strings` is on line 2, but after a 
change in line 1. Maybe sort the output?

I'm not against this procedure, but also not in favor. :)



Comment at: clang/docs/tools/dump_format_style.py:26
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:

FederAndInk wrote:
> MyDeveloperDay wrote:
> > This failed for me with invalid syntax
> Oh, ok, sorry, I might be using to recent python features, I'll remove type 
> specifier, what is the recommended python version to use?
> Oh, ok, sorry, I might be using to recent python features, I'll remove type 
> specifier, what is the recommended python version to use?

That **I** can not answer. I run
```$ python --version
Python 3.9.5
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:26
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:

MyDeveloperDay wrote:
> This failed for me with invalid syntax
Oh, ok, sorry, I might be using to recent python features, I'll remove type 
specifier, what is the recommended python version to use?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:26
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:

This failed for me with invalid syntax


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 369431.
FederAndInk added a comment.

generate plurals, for now supporting -y ending (-y to -ies/-ys), track 
generated plurals and show new ones to be checked


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/docs/tools/plurals.txt
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/plurals.txt
===
--- /dev/null
+++ clang/docs/tools/plurals.txt
@@ -0,0 +1,3 @@
+Strings
+IncludeCategories
+RawStringFormats
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -6,18 +6,54 @@
 import collections
 import os
 import re
+import inspect
+import subprocess
 
 CLANG_DIR = os.path.join(os.path.dirname(__file__), '../..')
 FORMAT_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Format/Format.h')
 INCLUDE_STYLE_FILE = os.path.join(CLANG_DIR, 'include/clang/Tooling/Inclusions/IncludeStyle.h')
 DOC_FILE = os.path.join(CLANG_DIR, 'docs/ClangFormatStyleOptions.rst')
 
+PLURAL_FILE = os.path.join(os.path.dirname(__file__), 'plurals.txt')
+subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE])
+plurals = set(open(PLURAL_FILE).read().splitlines())
 
 def substitute(text, tag, contents):
   replacement = '\n.. START_%s\n\n%s\n\n.. END_%s\n' % (tag, contents, tag)
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def register_plural(singular: str, plural: str):
+  if plural not in plurals:
+plurals.add(plural)
+open(PLURAL_FILE, 'a').write(plural + '\n')
+cf = inspect.currentframe()
+print(f'{__file__}:{cf.f_back.f_lineno} check if plural of {singular} is {plural}')
+  return plural
+
+def pluralize(word: str):
+  if len(word) >= 2 and word[-1] == 'y' and word[-2].lower() not in 'aeiou':
+return register_plural(word, word[:-1] + 'ies')
+  else:
+return register_plural(word, word + 's')
+
+
+def to_yaml_type(typestr: str):
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+  
+  [subtype, napplied] = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + pluralize(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +76,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +121,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** (``string``)
+**BasedOnStyle** (``String``)
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration
@@ -166,7 +166,7 @@
 
 .. START_FORMAT_STYLE_OPTIONS
 
-**AccessModifierOffset** (``int``)
+**AccessModifierOffset** (``Integer``)
   The extra indent or outdent of access modifiers, e.g. ``public:``.
 
 **AlignAfterOpenBracket** (``BracketAlignmentStyle``)
@@ -619,7 +619,7 @@
 
 
 
-**AlignTrailingComments** (``bool``)
+**AlignTrailingComments** (``Boolean``)
   If 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

Ok, here is my proposal for plurals, I have some ideas, I think the safest/most 
complete would be to have a file tracking generated plurals and tell the user 
of the script to check newly generated plurals then add them to git, this would 
also allow reviewers to see new plurals generated. I'll put the updated 
revision of my idea soon.

Another idea without tracking plurals would be to show the list of generated 
`word -> plural` but I think it would add noise over time and wouldn't add 
value, but if we don't want to add a plurals.txt file in git it would be a 
possibility.

And again, I don't really understand if we are allowed or not to pull in a 
dependency such as pluralizer or inflect, this would be another idea


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:23
+  plurals = {
+'IncludeCategory': 'IncludeCategories'
+  }

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > FederAndInk wrote:
> > > HazardyKnusperkeks wrote:
> > > > Could you not just check if there is a y at the end and replace it with 
> > > > ies, otherweise add an s?
> > > Well, I thought about it, but then what about: whish -> whishes, leaf -> 
> > > leaves, ... and irregulars? That's why I brought up the idea about using 
> > > python inflect. Do you think it's enough for now to replace y -> ies and 
> > > put an 's' to the others?
> > I'm okay with either way, in both cases there comes a time where someone 
> > must pay attention to add something here. We just have to look carefully in 
> > the review.
> it would be nice if in the event of a missing plural it complained.
But that would mean we have to add each plural here. So every new list, which 
is not of strings, would most likely also needed to be added here. I think that 
is too much, but then again there are not that much lists, so maybe it's worth 
the work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:23
+  plurals = {
+'IncludeCategory': 'IncludeCategories'
+  }

HazardyKnusperkeks wrote:
> FederAndInk wrote:
> > HazardyKnusperkeks wrote:
> > > Could you not just check if there is a y at the end and replace it with 
> > > ies, otherweise add an s?
> > Well, I thought about it, but then what about: whish -> whishes, leaf -> 
> > leaves, ... and irregulars? That's why I brought up the idea about using 
> > python inflect. Do you think it's enough for now to replace y -> ies and 
> > put an 's' to the others?
> I'm okay with either way, in both cases there comes a time where someone must 
> pay attention to add something here. We just have to look carefully in the 
> review.
it would be nice if in the event of a missing plural it complained.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:23
+  plurals = {
+'IncludeCategory': 'IncludeCategories'
+  }

FederAndInk wrote:
> HazardyKnusperkeks wrote:
> > Could you not just check if there is a y at the end and replace it with 
> > ies, otherweise add an s?
> Well, I thought about it, but then what about: whish -> whishes, leaf -> 
> leaves, ... and irregulars? That's why I brought up the idea about using 
> python inflect. Do you think it's enough for now to replace y -> ies and put 
> an 's' to the others?
I'm okay with either way, in both cases there comes a time where someone must 
pay attention to add something here. We just have to look carefully in the 
review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:23
+  plurals = {
+'IncludeCategory': 'IncludeCategories'
+  }

HazardyKnusperkeks wrote:
> Could you not just check if there is a y at the end and replace it with ies, 
> otherweise add an s?
Well, I thought about it, but then what about: whish -> whishes, leaf -> 
leaves, ... and irregulars? That's why I brought up the idea about using python 
inflect. Do you think it's enough for now to replace y -> ies and put an 's' to 
the others?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

Looks good and I agree with the choices.




Comment at: clang/docs/tools/dump_format_style.py:23
+  plurals = {
+'IncludeCategory': 'IncludeCategories'
+  }

Could you not just check if there is a y at the end and replace it with ies, 
otherweise add an s?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

> look at the html

Done, thanks

> I assume you don't have commit access, we'll need your name and email address

Here it is: Ludovic Jozeau  it should also be on my 
profile

> but if you think you might like to work on some other things, it might be 
> worth get ting the  "getting commit access" process started.

Yeah, after reading 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access, I think 
I'll wait until I do another contribution. :)

> I think its always good to wait to give others some chance to comment before 
> we commit, as we are all in different timezones.

Ok, that's fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D108765#2969036 , @FederAndInk 
wrote:

> For now, I handle plural manually, but it can be changed, I also kept 
> Unsigned, what are your thoughts?



  I think thats ok for now, did you try building the file with sphinx-build, I 
run
  
  `/usr/bin/sphinx-build -n ./docs ./html`
  
  Then go and look at the html

> Thanks for being so kind and responsive, it's really great to work on that :) 
> as it is my first contribution to clang.



  That is how it should be right ;-).
  
  I assume you don't have commit access, we'll need your name and email address 
if you want one of us to land it, so we can accredit you with the contribution, 
but if you think you might like to work on some other things, it might be worth 
get ting the  "getting commit access" process started.
  
  I think its always good to wait to give others some chance to comment before 
we commit, as we are all in different timezones.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

LGTM, thanks for adding that and fixing the spelling mistake, let the others 
have time to chip in.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

For now, I handle plural manually, but it can be changed, I also kept Unsigned, 
what are your thoughts?

Thanks for being so kind and responsive, it's really great to work on that :) 
as it is my first contribution to clang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk updated this revision to Diff 369081.
FederAndInk added a comment.

Use yaml type style for clang-format documentation (`String`, `Integer`, `List 
of Strings`, ...) instead of c++ types.

Fix typo in clang/Format/Format.h

Regenarate ClangFormatStyleOptions.rst


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3083,7 +3083,7 @@
 /// ForEach and If macros. This is useful in projects where ForEach/If
 /// macros are treated as function calls instead of control statements.
 /// ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
-/// backward compatability.
+/// backward compatibility.
 /// \code
 ///void f() {
 ///  Q_FOREACH(...) {
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -18,6 +18,30 @@
   pattern = r'\n\.\. START_%s\n.*\n\.\. END_%s\n' % (tag, tag)
   return re.sub(pattern, '%s', text, flags=re.S) % replacement
 
+def plural(word):
+  plurals = {
+'IncludeCategory': 'IncludeCategories'
+  }
+
+  return plurals.get(word, word + 's')
+
+def to_yaml_type(typestr):
+  typestr = str(typestr)
+  if typestr == 'bool':
+return 'Boolean'
+  elif typestr == 'int':
+return 'Integer'
+  elif typestr == 'unsigned':
+return 'Unsigned'
+  elif typestr == 'std::string':
+return 'String'
+  
+  [subtype, napplied] = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
+  if napplied == 1:
+return 'List of ' + plural(to_yaml_type(subtype))
+
+  return typestr
+
 def doxygen2rst(text):
   text = re.sub(r'\s*(.*?)\s*<\/tt>', r'``\1``', text)
   text = re.sub(r'\\c ([^ ,;\.]+)', r'``\1``', text)
@@ -40,7 +64,7 @@
 self.nested_struct = None
 
   def __str__(self):
-s = '**%s** (``%s``)\n%s' % (self.name, self.type,
+s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
  doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
@@ -85,7 +109,7 @@
 self.type = enumtype
 
   def __str__(self):
-s = '\n* ``%s %s``\n%s' % (self.type, self.name,
+s = '\n* ``%s %s``\n%s' % (to_yaml_type(self.type), self.name,
  doxygen2rst(indent(self.comment, 2)))
 s += indent('\nPossible values:\n\n', 2)
 s += indent('\n'.join(map(str, self.values)),2)
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** (``string``)
+**BasedOnStyle** (``String``)
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration
@@ -166,7 +166,7 @@
 
 .. START_FORMAT_STYLE_OPTIONS
 
-**AccessModifierOffset** (``int``)
+**AccessModifierOffset** (``Integer``)
   The extra indent or outdent of access modifiers, e.g. ``public:``.
 
 **AlignAfterOpenBracket** (``BracketAlignmentStyle``)
@@ -619,7 +619,7 @@
 
 
 
-**AlignTrailingComments** (``bool``)
+**AlignTrailingComments** (``Boolean``)
   If ``true``, aligns trailing comments.
 
   .. code-block:: c++
@@ -628,7 +628,7 @@
 int a; // My comment a  vs. int a; // My comment a
 int b = 2; // comment  bint b = 2; // comment about b
 
-**AllowAllArgumentsOnNextLine** (``bool``)
+**AllowAllArgumentsOnNextLine** (``Boolean``)
   If a function call or braced initializer list doesn't fit on a
   line, allow putting all arguments onto the next line, even if
   ``BinPackArguments`` is ``false``.
@@ -645,7 +645,7 @@
  c,
  d);
 
-**AllowAllConstructorInitializersOnNextLine** (``bool``)
+**AllowAllConstructorInitializersOnNextLine** (``Boolean``)
   If a constructor definition with a member initializer list doesn't
   fit on a single line, allow putting all member initializers onto the next
   line, if ```ConstructorInitializerAllOnOneLineOrOnePerLine``` is true.
@@ -663,7 +663,7 @@
 member0(0),
 member1(2) {}
 
-**AllowAllParametersOfDeclarationOnNextLine** (``bool``)
+**AllowAllParametersOfDeclarationOnNextLine** (``Boolean``)
   If the function declaration doesn't fit on a line,
   allow putting all parameters of a function declaration onto
   the next line even if 

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

Ok, well, the reason I proposed this patch in the first place was that I have 
been working on a `.clang-format` schema (https://json-schema.org/) :) and I 
spotted the inconsistency. I checked, and clang-format reports an error if we 
give a negative value to an option expecting an unsigned. In the schema I am 
able to specify a minimum and I think it's appropriate to give the information 
to the user that it expect a positive/unsigned integer, what do you think?

Also, interesting question, how do we want to handle plural, as the formulation 
'List of Types'  introduces it. If we do it manually, it won't scale. We could 
include a dependency in python to something like inflect 
. I'll upload a new patch soon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm good with words like `List of Strings` but I don't think we need `Enum`

`unsigned`  I think Integer, I'm not sure what the code is even going to do if 
you supply a -ve, Warn I hope! (there's contribution idea number 2 for you 
right there!) ;-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

What about: `unsigned`? And enum or struct types such as 
`BracketAlignmentStyle`, `ArrayInitializerAlignmentStyle`, ...?  Should we add 
something like: `Enum BracketAlignmentStyle` and `Dictionnary BraceWrapping`?

The complete list:

  53 bool  -> Boolean
  18 unsigned  -> ?
  9 std::vector   -> List of Strings
  5 std::string-> String
  4 AlignConsecutiveStyle  -> ?
  2 int-> Integer
  1 UseTabStyle-> ?
  1 TrailingCommaStyle -> ?
  1 string -> String
  1 std::vector   -> ?
  1 std::vector   -> ?...
  1 SpacesInLineComment
  1 SpacesInAnglesStyle
  1 SpaceBeforeParensOptions
  1 SpaceAroundPointerQualifiersStyle
  1 SortJavaStaticImportOptions
  1 SortIncludesOptions
  1 ShortLambdaStyle
  1 ShortIfStyle
  1 ShortFunctionStyle
  1 ShortBlockStyle
  1 ReturnTypeBreakingStyle
  1 ReferenceAlignmentStyle
  1 PPDirectiveIndentStyle
  1 PointerAlignmentStyle
  1 OperandAlignmentStyle
  1 NamespaceIndentationKind
  1 LanguageStandard
  1 LanguageKind
  1 LambdaBodyIndentationKind
  1 JavaScriptQuoteStyle
  1 IndentExternBlockStyle
  1 IncludeBlocksStyle
  1 EscapedNewlineAlignmentStyle
  1 EmptyLineBeforeAccessModifierStyle
  1 EmptyLineAfterAccessModifierStyle
  1 DefinitionReturnTypeBreakingStyle
  1 BreakTemplateDeclarationsStyle
  1 BreakInheritanceListStyle
  1 BreakConstructorInitializersStyle
  1 BracketAlignmentStyle
  1 BraceWrappingFlags
  1 BraceBreakingStyle
  1 BitFieldColonSpacingStyle
  1 BinPackStyle
  1 BinaryOperatorStyle
  1 ArrayInitializerAlignmentStyle


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added a comment.

As the one who wrote that:

1. Yes that part is not auto generated, because it is not in the `format.h`.
2. I'm more in favor of removing the `std::` from the documentation.
3. It is for the YAML documentation, then stick with the YAML names: `List of 
Strings` is my proposal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added subscribers: HazardyKnusperkeks, owenpan, krasimir, 
sammccall, curdeius, klimek.
MyDeveloperDay added a comment.

In D108765#2967363 , @FederAndInk 
wrote:

> Thank you for your explanations, I understand now.
>
> But as I look into `clang/docs/tools/dump_format_style.py` I see that it does 
> not entirely generate `clang/docs/ClangFormatStyleOptions.rst` it replaces 
> the lines between `{START,END}_FORMAT_STYLE_OPTIONS`
>
> I understand your point, but as of now, the inconsistency comes from the part 
> that is not auto-generated, are you suggesting editing `dump_format_style.py` 
> to have simpler types such as `string`? Then how should we replace 
> `std::vector`? Something like `Type[]` e.g. `string[]`?
>
> Or maybe we should first include `BasedOnStyle` into `dump_format_style.py`. 
> Then take care of how to render types?
>
> What do you suggest? I am genuinely asking, as I really don't know what would 
> be the best way to do things. Maybe we should include other people? I don't 
> really know who to add as reviewers for that, but I think, the way to show 
> types, should be discussed?
>
> As for detailing `RawStringFormat`, it wasn't the purpose of this patch, and 
> maybe it should have its own?

You are correct the file isn't 100% generated and some of it comes from another 
.h file too.

But now we have you interesting in making a contribution which you clearly are 
lets think about how we might do this.

To hook into the clang-format team I always recommend adding the #clang-format 
 project, (which I added to this 
review), but also I recommend passing the review via @krasimir , 
@HazardyKnusperkeks , @curdeius there are some others who are hear often like 
@owenpan and @sammccall and of course @klimek (who started all this). Please 
also of course add me @MyDeveloperDay I try to check the reviews daily as one 
of my frustrations was not being able to get things reviewed so I try to be 
pretty active.

From my perspective I do like the idea of substituting out the `std::string` 
and `std::vector` for something like `string[]` how about we start with 
something simple like trying to fix the cases for `AttributeMacros 
(std::vector)` maybe with just simple substitution.

We can pass that via the rest of the team and see what they feel even if we 
ultimately have both

AttributeMacros (in configuration `string[]`)

or something like that, I'm not con convinced anyone is using this 
documentation to know its a std::vector!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk added a comment.

Thank you for your explanations, I understand now.

But as I look into `clang/docs/tools/dump_format_style.py` I see that it does 
not entirely generate `clang/docs/ClangFormatStyleOptions.rst` it replaces the 
lines between `{START,END}_FORMAT_STYLE_OPTIONS`

I understand your point, but as of now, the inconsistency comes from the part 
that is not auto-generated, are you suggesting editing `dump_format_style.py` 
to have simpler types such as `string`? Then how should we replace 
`std::vector`? Something like `Type[]` e.g. `string[]`?

Or maybe we should first include `BasedOnStyle` into `dump_format_style.py`. 
Then take care of how to render types?

What do you suggest? I am genuinely asking, as I really don't know what would 
be the best way to do things. Maybe we should include other people? I don't 
really know who to add as reviewers for that, but I think, the way to show 
types, should be discussed?

As for detailing `RawStringFormat`, it wasn't the purpose of this patch, and 
maybe it should have its own?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

Thank you for your submission, but...

1. This is not the way to change this file, its autogenerated from 
clang/include/Format/Format.h using  clang/docs/tools/dump_format_style.py
2. Why do you think it should be std::string? To be honest this file pretty 
much describes the `YAML` file format of `.clang-format` so actually I would 
suggest it saying string was more correct

I think the main problem is options like this:

F18679590: image.png 

Here we say its a `std::vector` which doesn't really tell you 
much, as its actually just a `YAML` array of `RawStringFormat` records without 
actually telling you what a `RawStringFormat` record contains

  RawStringFormats:
- Language: TextProto
Delimiters:
  - 'pb'
  - 'proto'
EnclosingFunctions:
  - 'PARSE_TEXT_PROTO'
BasedOnStyle: google
- Language: Cpp
Delimiters:
  - 'cc'
  - 'cpp'
BasedOnStyle: llvm
CanonicalDelimiter: 'cc'

So whilst I see that you were being consistent I kind of feel its in the wrong 
direction, we should be moving away from using `C++` names here but more 
explaining what the `YAML` should look like.

Alas the C++ in Format.h is important because the types are needed because we 
are generating the documentation directly from the code. But in most of places 
we are using an enumeration like this one.

F18679643: image.png 

we are not saying  `clang::FormatStyle::RefernceAlignmentStyle` and I'm not 
convinced we'd want to

I hope that helps, but thank you for the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108765/new/

https://reviews.llvm.org/D108765

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

[Ludovic Jozeau via Phabricator via cfe-commits]

FederAndInk created this revision.
FederAndInk requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix little inconsistency and use `std::string` (which is used everywhere
else) instead of `string`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108765

Files:
  clang/docs/ClangFormatStyleOptions.rst


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** (``string``)
+**BasedOnStyle** (``std::string``)
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -125,7 +125,7 @@
 the configuration (without a prefix: ``Auto``).
 
 
-**BasedOnStyle** (``string``)
+**BasedOnStyle** (``std::string``)
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits