Skip to content

Comments

Dont highlight tab/space errors in the BTHelp buffers#3189

Merged
dmaluka merged 1 commit intomicro-editor:masterfrom
dustdfg:fix/help-dont-highlight-tab-space-errors
Mar 19, 2024
Merged

Dont highlight tab/space errors in the BTHelp buffers#3189
dmaluka merged 1 commit intomicro-editor:masterfrom
dustdfg:fix/help-dont-highlight-tab-space-errors

Conversation

@dustdfg
Copy link
Contributor

@dustdfg dustdfg commented Mar 18, 2024

No description provided.

Signed-off-by: Yevhen Babiichuk (DustDFG) <dfgdust@gmail.com>
@dmaluka
Copy link
Collaborator

dmaluka commented Mar 18, 2024

NACK. We should rather fix those errors than hide them.

@dustdfg
Copy link
Contributor Author

dustdfg commented Mar 18, 2024

NACK. We should rather fix those errors than hide them.

I agree about trialing spaces but... Not about leading spaces... Why? Do you like such help?

screen-1710768429

@Andriamanitra
Copy link
Contributor

I agree about trialing spaces but... Not about leading spaces... Why? Do you like such help?

I think the leading spaces/tabs is only highlighted when spaces and tabs are mixed. As long as all leading whitespace is converted to spaces it should be fine.

@dmaluka
Copy link
Collaborator

dmaluka commented Mar 18, 2024

I agree about trialing spaces but... Not about leading spaces... Why? Do you like such help?
...

Hmm, good point. I bet you have tabstospaces set to false and you are not using the detectindent plugin, right? Yeah, we should probably address this case. We can force hltaberrors to false for BTHelp, like you did, or alternatively we can force tabstospaces to true for BTHelp, so that if hltaberrors is enabled, it doesn't erroneously highlight leading spaces, but still highlights erroneous leading tabs. Basically, both ways seem ok to me.

@dustdfg
Copy link
Contributor Author

dustdfg commented Mar 18, 2024

Hmm, good point. I bet you have tabstospaces set to false and you are not using the detectindent plugin, right?

Yeah

Yeah, we should probably address this case. We can force hltaberrors to false for BTHelp, like you did, or alternatively we can force tabstospaces to true for BTHelp, so that if hltaberrors is enabled, it doesn't erroneously highlight leading spaces, but still highlights erroneous leading tabs. Basically, both ways seem ok to me.

As a usual user don't want to see any errors of the program which doesn't affect me in any way. I just want to read help pages. I read them because I am not advanced which means that I don't need to see any red thing anyway. Yeah it could help the developer to notice them but if they are noticed and fixed and I can't update?

I disabled errors and not used tabstospaces because end user doesn't need this info... Anyway is there spaces or tabs or there leading spaces? User doesn't care, don't make users to see something red to scare them. If we use tabstospaces it will be not so user friendly becuase in some sitaution user can see error (if developer had made a mistake) so I think my solution is better

@JoeKar
Copy link
Member

JoeKar commented Mar 18, 2024

screen-1710768429

Especially the options.md currently is a pain in the ***. I was already changing it within a different PR from me not really related the same and it was correctly rejected (due to unrelated changes), but we need to fix this mess. It will trigger neurotics. 😄
Wasn't there already a PR to address this? -> Now #3193 exists.

@dustdfg
Copy link
Contributor Author

dustdfg commented Mar 18, 2024

Wasn't there already a PR to address this?

The tab/space errors PR was merged recently. Maybe there were PR's like #3190 but I don't think there were PR's about disabling the unmerged for that moment feature

I was already changing it within a different PR not really related to that and it was correctly rejected, but we need to fix this mess.

0_o changing unmerged feature?

@JoeKar
Copy link
Member

JoeKar commented Mar 18, 2024

Maybe there were PR's like #3190 but I don't think there were PR's about disabling the unmerged for that moment feature

I'm fully lost and duplicated (while extending) your #3190. I need to bring my childs to bed first, then I'm more relaxed. 😄

@dmaluka
Copy link
Collaborator

dmaluka commented Mar 18, 2024

For the record, the user is free not to enable hltaberrors or hltrailingws. They are disabled by default.

(BTW hltaberrors is actually a tricky heuristic, so it may not be wanted by some users (e.g. if the user is working with code that uses tab indentation, yet for some reason the user wants to have tabstospaces enabled, and doesn't want to see those red colors as a result of that). But this behavior of hltaberrors is documented, and it is disabled by default, so I suppose it's ok: if the user enables hltaberrors, he knows what he is doing.)

Enforcing disabling hltaberrors for BTHelp makes sense, as I've already agreed. As for enforcing disabling hltrailingws, I don't see a necessity in it, once we clean up the existing whitespace errors in the help pages, and don't merge PRs introducing new errors. But if you insist on disabling hltrailingws as well, fine.

@dustdfg
Copy link
Contributor Author

dustdfg commented Mar 19, 2024

Ok... I see two options:

  1. Force disable trailing spaces
  2. Force enable trailing spaces (because if we just omit forcing it someone will have it and someone will not have it...)

One more thing. When someone edits original file. The settings of BTHelp buffers aren't applied so the maintainers will need to catch the mistakes so it doesn't really matter much...

@dmaluka dmaluka merged commit b518bda into micro-editor:master Mar 19, 2024
@dmaluka
Copy link
Collaborator

dmaluka commented Mar 19, 2024

No need to endlessly discuss such trivia, I think. Merged. :)

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.

4 participants