Skip to content

add minimal line count - #7

Merged
timakro merged 1 commit into
timakro:mainfrom
Konfekt:min-line-count
Mar 5, 2024
Merged

add minimal line count#7
timakro merged 1 commit into
timakro:mainfrom
Konfekt:min-line-count

Conversation

@Konfekt

@Konfekt Konfekt commented Mar 3, 2024

Copy link
Copy Markdown
Contributor

addresses #6

@Konfekt
Konfekt force-pushed the min-line-count branch 3 times, most recently from 5e13eb8 to 4369178 Compare March 3, 2024 20:27
@timakro

timakro commented Mar 4, 2024

Copy link
Copy Markdown
Owner

I don't think this is a good idea. When there is a single indented line in a file it's more likely than not the true indentation level of the file. Sure, there can be outliers, but without being more clever the best thing we can do is assume the most likely scenario which is that the indentation we see is the true indentation.

@Konfekt

Konfekt commented Mar 4, 2024

Copy link
Copy Markdown
Contributor Author

When there are only one or two indented lines, it is rather unlikely that an indent of 5 or 7, say, was indented by the user, but rather accidental. Therefore, only accept common values to keep out of the way.

@timakro

timakro commented Mar 4, 2024

Copy link
Copy Markdown
Owner

This gives me the following idea: if there are only one or two indented lines, only accept common indents like 2, 4 (maybe 8). This would solve both #4 and #6. What do you think?

@Konfekt

Konfekt commented Mar 4, 2024

Copy link
Copy Markdown
Contributor Author

Yes, I agree. The two line limit is arbitrary, of course.
Still, indent increases > 8 should be disregarded as well, as your example in #5 shows.

@timakro
timakro merged commit d63c811 into timakro:main Mar 5, 2024
@timakro

timakro commented Mar 5, 2024

Copy link
Copy Markdown
Owner

Merged! I don't think disregarding indentations > 8 is necessary as this already covers the example in #5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants