search

zyedidia / micro

A modern and intuitive terminal-based text editor
https://micro-editor.github.io
MIT License
24.42k stars 1.16k forks source link

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

Closed dustdfg closed 3 months ago

dmaluka commented 3 months ago

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

dustdfg commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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. :smile: Wasn't there already a PR to address this? -> Now #3193 exists.

dustdfg commented 3 months ago

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 commented 3 months ago

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. :smile:

dmaluka commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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