Skip to content

Commit

Permalink
Enable Ktlint block-comment-initial-star-alignment rule (#5517)
Browse files Browse the repository at this point in the history
  • Loading branch information
FloEdelmann authored Mar 2, 2024
1 parent ddba17c commit fa8da80
Show file tree
Hide file tree
Showing 14 changed files with 195 additions and 186 deletions.
1 change: 0 additions & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ ktlint_standard_argument-list-wrapping = disabled
ktlint_standard_backing-property-naming = disabled
ktlint_standard_blank-line-before-declaration = disabled
ktlint_standard_blank-line-between-when-conditions = disabled
ktlint_standard_block-comment-initial-star-alignment = disabled
ktlint_standard_chain-wrapping = disabled
ktlint_standard_comment-wrapping = disabled
ktlint_standard_function-naming = disabled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,17 @@ private fun isNodeGeometrySubstantiallyDifferent(node: Node, newNode: Node) =
node.position.distanceTo(newNode.position) > 30

private fun isWayGeometrySubstantiallyDifferent(way: Way, newWay: Way): Boolean {
/* if the first or last node is different, it means that the way has either been extended or
shortened at one end, which is counted as being substantial:
If for example the surveyor has been asked to determine something for a certain way
and this way is now longer, his answer does not apply to the whole way anymore, so that
is an unsolvable conflict.
/*
if the first or last node is different, it means that the way has either been extended or
shortened at one end, which is counted as being substantial:
If for example the surveyor has been asked to determine something for a certain way
and this way is now longer, his answer does not apply to the whole way anymore, so that
is an unsolvable conflict.
Furthermore, if the original way's end node id is negative (=has just been created in this
app), don't do that check for that node.
See https://github.com/streetcomplete/StreetComplete/issues/2800
*/
Furthermore, if the original way's end node id is negative (=has just been created in this
app), don't do that check for that node.
See https://github.com/streetcomplete/StreetComplete/issues/2800
*/
val firstNodeId = way.nodeIds.first()
if (firstNodeId >= 0) {
if (firstNodeId != newWay.nodeIds.first()) return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@ fun Element.changesApplied(changes: StringMapChanges): Element {
} catch (e: IllegalStateException) {
throw ConflictException("Conflict while applying the changes")
} catch (e: IllegalArgumentException) {
/* There is a max key/value length limit of 255 characters in OSM. If we reach this
point, it means the UI did permit an input of more than that. So, we have to catch
this here latest.
The UI should prevent this in the first place, at least
for free-text input. For structured input, like opening hours, it is another matter
because it's awkward to explain to a non-technical user this technical limitation
/*
There is a max key/value length limit of 255 characters in OSM. If we reach this
point, it means the UI did permit an input of more than that. So, we have to catch
this here latest.
The UI should prevent this in the first place, at least
for free-text input. For structured input, like opening hours, it is another matter
because it's awkward to explain to a non-technical user this technical limitation
See also https://github.com/openstreetmap/openstreetmap-website/issues/2025
*/
See also https://github.com/openstreetmap/openstreetmap-website/issues/2025
*/
throw ConflictException("Key or value is too long")
}
return this.copy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,21 +198,22 @@ private fun Note.shouldShowAsQuest(
// don't show notes hidden by user
if (id in blockedNoteIds) return false

/* don't show notes where user replied last unless he wrote a survey required marker */
// don't show notes where user replied last unless he wrote a survey required marker
if (comments.last().isReplyFromUser(userId)
&& !comments.last().containsSurveyRequiredMarker()
) {
return false
}

/* newly created notes by user should not be shown if it was both created in this app and has no
replies yet */
// newly created notes by user should not be shown if it was both created in this app and has no replies yet
if (probablyCreatedByUserInThisApp(userId) && !hasReplies) return false

/* many notes are created to report problems on the map that cannot be resolved
* through an on-site survey.
* Likely, if something is posed as a question, the reporter expects someone to
* answer/comment on it, possibly an information on-site is missing, so let's only show these */
/*
many notes are created to report problems on the map that cannot be resolved
through an on-site survey.
Likely, if something is posed as a question, the reporter expects someone to
answer/comment on it, possibly an information on-site is missing, so let's only show these
*/
if (showOnlyNotesPhrasedAsQuestions
&& !probablyContainsQuestion()
&& !containsSurveyRequiredMarker()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@ import de.westnordost.streetcomplete.osm.updateCheckDateForKey

fun LeftAndRightSidewalk.applyTo(tags: Tags) {
if (left == null && right == null) return
/* for being able to modify only one side (e.g. `left` is null while `right` is not null),
first the conflated and merged sidewalk values (sidewalk=both and sidewalk:both=yes etc.)
need to be separated.
So even if there is an invalid value such as sidewalk=narrow but only the right side
is modified to "yes" while the left side is not touched, it means that in the end, the
invalid value must still be in the end result, like this:
- sidewalk:left=narrow
- sidewalk:right=yes
First separating the values and then later conflating them again, if possible, solves this.
*/
/*
for being able to modify only one side (e.g. `left` is null while `right` is not null),
first the conflated and merged sidewalk values (sidewalk=both and sidewalk:both=yes etc.)
need to be separated.
So even if there is an invalid value such as sidewalk=narrow but only the right side
is modified to "yes" while the left side is not touched, it means that in the end, the
invalid value must still be in the end result, like this:
- sidewalk:left=narrow
- sidewalk:right=yes
First separating the values and then later conflating them again, if possible, solves this.
*/

/* NOT expanding the bare tag, because the bare tag follows a different schema
E.g. sidewalk=both != sidewalk:left=both + sidewalk:right=both
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,29 +51,30 @@ class SeparateCyclewayForm : AImageSelectOverlayForm<SeparateCycleway>() {

val cycleway = parseSeparateCycleway(element!!.tags)

/* Not displaying bicycle=yes and bicycle=no on footways and treating it the same because
whether riding a bike on a footway is allowed by default (without requiring signs) or
only under certain conditions (e.g. certain minimum width of sidewalk) is very much
dependent on the country or state one is in.
Hence, it is not verifiable well for the on-site surveyor: If there is no sign that
specifically allows or forbids cycling on a footway, the user is left with his loose
(mis)understanding of the local legislation to decide. After all, bicycle=yes/no
is (usually) nothing physical, but merely describes what is legal. It is in that sense
then not information surveyable on-the-ground, unless specifically signed.
bicycle=yes/no does however not make a statement about from where this info is derived.
So, from an on-site surveyor point of view, it is always better to record what is signed,
instead of what follows from that signage.
Signage, however, is out of scope of this overlay because while the physical presence of
a cycleway can be validated at a glance, the presence of a sign requires to walk a bit up
or down the street in order to find (or not find) a sign.
More importantly, at the time of writing, there is no way to tag the information that a
bicycle=* access restriction is derived from the presence of a sign. This however is a
prerequisite for it being displayed as a selectable option due to the reasons stated
above.
*/
/*
Not displaying bicycle=yes and bicycle=no on footways and treating it the same because
whether riding a bike on a footway is allowed by default (without requiring signs) or
only under certain conditions (e.g. certain minimum width of sidewalk) is very much
dependent on the country or state one is in.
Hence, it is not verifiable well for the on-site surveyor: If there is no sign that
specifically allows or forbids cycling on a footway, the user is left with his loose
(mis)understanding of the local legislation to decide. After all, bicycle=yes/no
is (usually) nothing physical, but merely describes what is legal. It is in that sense
then not information surveyable on-the-ground, unless specifically signed.
bicycle=yes/no does however not make a statement about from where this info is derived.
So, from an on-site surveyor point of view, it is always better to record what is signed,
instead of what follows from that signage.
Signage, however, is out of scope of this overlay because while the physical presence of
a cycleway can be validated at a glance, the presence of a sign requires to walk a bit up
or down the street in order to find (or not find) a sign.
More importantly, at the time of writing, there is no way to tag the information that a
bicycle=* access restriction is derived from the presence of a sign. This however is a
prerequisite for it being displayed as a selectable option due to the reasons stated
above.
*/
currentCycleway = if (cycleway == NOT_ALLOWED || cycleway == ALLOWED_ON_FOOTWAY) NON_DESIGNATED else cycleway
selectedItem = currentCycleway?.asItem(countryInfo.isLeftHandTraffic)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,28 +220,28 @@ class SurfaceOverlayForm : AbstractOverlayForm() {
private fun createSegregatedAnswer(): AnswerItem? =
if (isSegregatedLayout) {
/*
No option to switch back to single surface. Removing info about separate cycleway is
too complicated.
No option to switch back to single surface. Removing info about separate cycleway is
too complicated.
Typically it requires editing not only surface info but also an access info as it
happens in cases where bicycle access is gone. May require also removal of
cycleway=separate, bicycle=use_sidepath from the road.
Typically it requires editing not only surface info but also an access info as it
happens in cases where bicycle access is gone. May require also removal of
cycleway=separate, bicycle=use_sidepath from the road.
And in cases where there is a segregated cycleway with the same surface as footway
then StreetComplete will anyway ask for cycleway:surface and footway:surface.
And in cases where there is a segregated cycleway with the same surface as footway
then StreetComplete will anyway ask for cycleway:surface and footway:surface.
Fortunately need for this change are really rare. Notes can be left as usual.
*/
Fortunately need for this change are really rare. Notes can be left as usual.
*/
null
} else if (isBothFootAndBicycleTraffic(element!!)) {
/*
Only where bicycle access is already present because adding bicycle access typically
requires adding proper access tags, interconnections with roads and often also other
geometry changes.
Only where bicycle access is already present because adding bicycle access typically
requires adding proper access tags, interconnections with roads and often also other
geometry changes.
In case where path is not clearly marked as carrying both foot and bicycle traffic
mapper can leave a note
*/
In case where path is not clearly marked as carrying both foot and bicycle traffic
mapper can leave a note
*/
AnswerItem(R.string.overlay_path_surface_segregated) {
// reset previous data
surfaceCtrl.value = originalSurface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,37 +209,37 @@ fun questTypeRegistry(
getFeature: (Element) -> Feature?,
) = QuestTypeRegistry(listOf(

/* The quest types are primarily sorted by how easy they can be solved:
1. quests that are solvable from a distance or while passing by (fast)
2. quests that require to be right in front of it (e.g. because it is small, you need to
look for it or read text)
3. quests that require some exploration or walking around to check (e.g. walking down the
whole road to find the cycleway is the same along the whole way)
4. quests that require to go inside, i.e. deviate from your walking route by a lot just
to solve the quest
5. quests that come in heaps (are spammy) come last: e.g. building type etc.
The ordering within this primary sort order shall be whatever is faster so solve first:
a. Yes/No quests, easy selections first,
b. number and text inputs later,
c. complex inputs (opening hours, ...) last. Quests that e.g. often require the way to be
split up first are in effect also slow to answer
The order can be watered down somewhat if it means that quests that usually apply to the
same elements are in direct succession because we want to avoid that users are half-done
answering all the quests for one element and then can't solve the last anymore because it
is visually obscured by another quest.
Finally, importance of the quest can still play a factor, but only secondarily.
---
Each quest is assigned an ordinal. This is used for serialization and is thus never changed,
even if the quest's order is changed or new quests are added somewhere in the middle. Each new
quest always gets a new sequential ordinal.
*/
/*
The quest types are primarily sorted by how easy they can be solved:
1. quests that are solvable from a distance or while passing by (fast)
2. quests that require to be right in front of it (e.g. because it is small, you need to
look for it or read text)
3. quests that require some exploration or walking around to check (e.g. walking down the
whole road to find the cycleway is the same along the whole way)
4. quests that require to go inside, i.e. deviate from your walking route by a lot just
to solve the quest
5. quests that come in heaps (are spammy) come last: e.g. building type etc.
The ordering within this primary sort order shall be whatever is faster so solve first:
a. Yes/No quests, easy selections first,
b. number and text inputs later,
c. complex inputs (opening hours, ...) last. Quests that e.g. often require the way to be
split up first are in effect also slow to answer
The order can be watered down somewhat if it means that quests that usually apply to the
same elements are in direct succession because we want to avoid that users are half-done
answering all the quests for one element and then can't solve the last anymore because it
is visually obscured by another quest.
Finally, importance of the quest can still play a factor, but only secondarily.
---
Each quest is assigned an ordinal. This is used for serialization and is thus never changed,
even if the quest's order is changed or new quests are added somewhere in the middle. Each new
quest always gets a new sequential ordinal.
*/

/* always first: notes - they mark a mistake in the data so potentially every quest for that
element is based on wrong data while the note is not resolved */
Expand Down Expand Up @@ -309,12 +309,14 @@ fun questTypeRegistry(
35 to AddRecyclingContainerMaterials(),

// kerbs
36 to AddKerbHeight(), /* deliberately before AddTactilePavingKerb:
* - Also should be visible while waiting to cross
* - Some people are not interpreting flush or lowered kerb as a kerb on their own,
* and would be confused about asking about tactile status on kerb without kerb
* but with this quest first they are OK with such interpretation
*/
36 to AddKerbHeight(),
/*
AddKerbHeight is deliberately before AddTactilePavingKerb:
- Also should be visible while waiting to cross
- Some people are not interpreting flush or lowered kerb as a kerb on their own,
and would be confused about asking about tactile status on kerb without kerb
but with this quest first they are OK with such interpretation
*/
37 to AddTactilePavingKerb(), // Paving can be completed while waiting to cross

// crossing quests: A little later because they are not all solvable from a distance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ class AddCrossingForm : AListQuestForm<CrossingAnswer>() {
TextItem(PROHIBITED, R.string.quest_crossing_prohibited),
)

/* PROHIBITED is not possible for sidewalks or crossings (=separately mapped sidewalk
/*
PROHIBITED is not possible for sidewalks or crossings (=separately mapped sidewalk
infrastructure) because if the crossing does not exist, it would require to also
delete/adapt the crossing ways, rather than just tagging crossing=no on the vertex.
Expand All @@ -27,7 +28,7 @@ class AddCrossingForm : AListQuestForm<CrossingAnswer>() {
NO on the other hand would be okay because crossing=informal would not require deleting
the crossing ways (I would say... it is in edge case...)
*/
*/
override fun onClickOk() {
if (checkedItem?.value == PROHIBITED && isOnSidewalkOrCrossing()) {
AlertDialog.Builder(requireContext())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ class AddOneway : OsmElementQuestType<OnewayAnswer> {
}

return onewayCandidates.filter {
/* ways that are simply at the border of the download bounding box are treated as if
they are dead ends. This is fine though, because it only leads to this quest not
showing up for those streets (which is better than the other way round)
*/
/*
ways that are simply at the border of the download bounding box are treated as if
they are dead ends. This is fine though, because it only leads to this quest not
showing up for those streets (which is better than the other way round)
*/
// check if the way has connections to other roads at both ends
(connectionCountByNodeIds[it.nodeIds.first()] ?: 0) > 1 &&
(connectionCountByNodeIds[it.nodeIds.last()] ?: 0) > 1
Expand Down
Loading

0 comments on commit fa8da80

Please sign in to comment.