Feature/issue 1508 semicolon extra and missing #1509
Conversation
alonthedark
changed the title
add empty check
.../com.e1c.v8codestyle.bsl.ui/src/com/e1c/v8codestyle/bsl/ui/qfix/MethodSemicolonExtraFix.java
Show resolved
Hide resolved
bundles/com.e1c.v8codestyle.bsl.ui/src/com/e1c/v8codestyle/bsl/ui/qfix/SemicolonMissingFix.java
Outdated
Show resolved
Hide resolved
bundles/com.e1c.v8codestyle.bsl/src/com/e1c/v8codestyle/bsl/check/messages.properties
Outdated
Show resolved
Hide resolved
Проверка на нахождение statement
contain -> contains
ReplaceEdit -> InsertEdit
| { | ||
| return null; | ||
| } | ||
| return new InsertEdit(node.getEndOffset(), ";"); //$NON-NLS-1$ |
There was a problem hiding this comment.
что-то у меня тут ошибка, в гите что ли правилось? не хорошо это
|
|
||
| public static String MethodTooManyPramsCheck_title; | ||
|
|
||
| public static String MethodSemicolonExtraCheck_Description; |
There was a problem hiding this comment.
в этом файле все с разделителем в 1 строку, я бы для общности сохранял практику, не знаю, кто придумал, но для однородности сохранил бы
|
|
||
| if (checkText.contains(";")) //$NON-NLS-1$ | ||
| { | ||
| resultAceptor.addIssue(Messages.MethodSemicolonExtraCheck_Issue, NAMED_ELEMENT__NAME); |
There was a problem hiding this comment.
я смотрю, что проверка ставится на сам метод, а не на точку с запятой, может лучше на точку, ну так бытсрее и понятнее видно, где проблема и что не так
| { | ||
| return null; | ||
| } | ||
| INode checkNode = node.getNextSibling(); |
There was a problem hiding this comment.
в аналогичном месте в самом check тут есть проверка, что checkNode не null
| IProgressMonitor monitor) | ||
| { | ||
| Method method = (Method)object; | ||
| List<Statement> allItems = BslUtil.allStatements(method); |
There was a problem hiding this comment.
я понимаю, что жаловались толкьо на ; в начале методов, но чисто по общности вот такой вариант тоже не очень хорош, можно отдельно со всеми обсудить будет ли править
| } | ||
| if (!checkText.contains(";") && !nodeText.isEmpty()) //$NON-NLS-1$ | ||
| { | ||
| resultAceptor.addIssue(Messages.SemicolonMissingCheck_Issue); |
There was a problem hiding this comment.
мне не нравится, что по сути выделяется весь statement, выглядит как по мне ужасно и вообще не понятно, что не так, плюс слишком много вхождений на 1 случай
| } | ||
|
|
||
| List<EObject> eObJects = statement.eContents(); | ||
| checkSemicolon(eObJects, resultAceptor); |
There was a problem hiding this comment.
неверно работает в этом случае
| } | ||
| } | ||
|
|
||
| private void checkSemicolon(List<EObject> eObjects, ResultAcceptor resultAceptor) |
There was a problem hiding this comment.
а в конфигурации документоборота вообще нашел очень странный случай с таким выделением
приложу еще и сам весь модуль
|
|
||
| private void checkSemicolon(List<EObject> eObjects, ResultAcceptor resultAceptor) | ||
| { | ||
| if (eObjects.isEmpty()) |
There was a problem hiding this comment.
по замерам производительности весьма ощутимо у меня вышло на документообороте, кажется, что не очень подход выбран, что надо проверять каждый statement в методе, может попробуем проверять сходу весь метод, а в нем только последние statement в каждом блоке statement.
Добавление пустых строк
2 participants
Что сделано
Чек-лист
Общее:
masterи нет конфликтовЕсли применимо:
Закрываемые задачи
Closes #1508