codecadwallader / codemaid

CodeMaid is an open source Visual Studio extension to cleanup and simplify our C#, C++, F#, VB, PHP, PowerShell, JSON, XAML, XML, ASP, HTML, CSS, LESS, SCSS, JavaScript and TypeScript coding.
http://www.codemaid.net
GNU Lesser General Public License v3.0
1.92k stars 361 forks source link

"Wrap comments at column" sometimes breaks line too soon and/or adds leading spaces (erroneously) #527

Open lsmith999999 opened 6 years ago

lsmith999999 commented 6 years ago

Environment

Description

Sometimes the "Wrap comments at column" feature breaks a line far too soon, or injects its own leading white space at the start of a line (erroneously). The problem only occurs every now again but I've seen it enough times now in the past year or two (since I started using CodeMaid) that I decided to notify you. The example below for instance demonstrates the problem (consistently). I'm assuming (maybe naively) that the particular code you need to review probably isn't too long (or complicated) so just double-checking it may turn up the problem. It's not a show-stopper (not a blocking issue) but does need correcting (probably low priority for you though).

Steps to recreate

WYSIWYG (see example below)

Current behavior

Given this (contrived) example which starts in column 1, and the setting is currently set at column 76 (note that no tab characters exist -> ASCII 9 - just spaces only -> ASCII 32 - and no trailing spaces exist on any line) :

// To be, or not to be, that is the question. Whether 'tis nobler in the mind to suffer the slings and // arrows of outrageous fortune, Or to take Arms against a Sea of troubles, And by opposing end // them: to die, to sleep No more; and by a sleep, to say we end the heart-ache, and the thousand // natural shocks that Flesh is heir to?

After hitting <Ctrl+M+N> this results:

// To be, or not to be, that is the question. Whether 'tis nobler in the mind // to suffer the slings and arrows of outrageous fortune, Or to take Arms // against a Sea of troubles, And by opposing end // them: to die, to sleep No more; and by a sleep, to say we end the // heart-ache, and the thousand natural shocks that Flesh is heir to?

Expected behavior

// To be, or not to be, that is the question. Whether 'tis nobler in the mind // to suffer the slings and arrows of outrageous fortune, Or to take Arms // against a Sea of troubles, And by opposing end them: to die, to sleep No // more; and by a sleep, to say we end the heart-ache, and the thousand // natural shocks that Flesh is heir to?

Note that trivial changes in the comments such as simply replacing the (one and only) colon ":" with a period "." eliminates the problem. In real comments I sometimes find the problem occurs when a hyphen "-" exists. It appears that you're treating some characters (punctuation?) in a special way. Presumably (I'm guessing) you're using some type of your own home-grown heuristics to do the wrapping, where certain punctuation factors in. The situation needs reviewing though.

lsmith999999 commented 6 years ago

After posting this I see that that the word "heart-ache" isn't being displayed correctly in the <Ctrl+M+N> version of my sample (an issue in my browser perhaps or maybe GitHub is reformatting it). In Visual Studio however it's (erroneously) preceded by 7 spaces (so it's part of the problem I'm trying to demonstrate).

codecadwallader commented 6 years ago

Thanks for reporting the issue. There is special logic that tries to identify "lists" within comments and so some special characters (e.g. colons, hyphens) will be treated differently. @willemduncan can you help elaborate or offer any workarounds? I didn't see an obvious setting that could be set to disable list behavior.

lsmith999999 commented 6 years ago

Ok, understood (thanks). List identification obviously needs some work but it's just an occasional nuisance only. At least you're aware of the situation (not complaining - your product is greatly appreciated!). On a (completely) separate issue, at some point (when I have the time), I might post some suggestions on enhancing this feature in general (which I use a lot). In particular, it would be great if comments could be wrapped using different column numbers on-the-fly, without having to change the "Wrap comments at column" each time (which may be useful as a default but slow and inconvenient when wrapping comments starting at different column locations - this often requires different values for this setting). I can see (for example) just highlighting the rows you want to wrap (using Alt+LeftMouseButton for instance - already built into Visual Studio), and then wrapping those highlighted rows only. The column to wrap at would simply be the end of the highlighted block itself (since it's fast and easy to drag this via Alt+LeftMouseButton). If "Alt" isn't depressed at the time however then the default column could just kick in. Try each to see what I mean (with "Alt" depressed and without). This system (or something similar) would be much more powerful and flexible (since you can quickly and easily control which rows to wrap and the column to wrap at). It will take a little thinking (not sure if this approach is always ergonomically correct), but it or something similar would be better IMHO. Anyway, food for thought. Thanks again.

w5l commented 5 years ago

I'm not sure what to do with this issue. It is indeed caused the list item detection, which will add indentation if the line wraps. In this case it is trigged by the word "them:" being on the first position. The rest of the line will then wrap but be indented by the list prefix length. One leading space, 6 chars for "them:, and one trailing space make the reported 7.

Can we close this issue and start a new one dealing with the more generic point of list item detection?

The easiest way out I see would be to only trigger list behavior when there are multiple (ie > 1) list items. But that breaks existing functionality, I know I use single item lists every now and then to visually highlight important points within a comment. Not a big issue to me if that ceased to work but opinions might vary.

On your point of the variable wrapping column: it's been suggested before (tried to find the related issue but I can;t find it right now). It's definitely on the wish list, and using vertical selection / column mode to indicate the desired width is a beautiful idea. Thanks!

lsmith999999 commented 5 years ago

Ok, thanks for the feedback. I honestly wouldn't worry about the "list item detection" for now. I'd have to review the situation anyway but the original problem I reported has always been just a minor (infrequent) nuisance only (no big deal - I can live with it). The wrapping issue is much more important for me personally (not sure about others but I suspect many would benefit from it). I use this frequently and the current design is very cumbersome/slow to use when you require different column settings all the time (respectfully). BTW, I accidentally referred to "Alt+LeftMouseWheel" to activate vertical/column selection in my original post, when I should have said "Alt+LeftMouseButton" (though it can also be activated using the keyboard). I'll correct this in my original post.

In any case, vertical/column selection does seems like a natural way to do it (thanks for the positive feedback), though it's unclear what issues there may be doing it this way, if any (I haven't thought out the details). At the top of my head however, I can see a system where, once the vertical/column area to wrap is selected, if you then simply click anywhere in the area with the left mouse button, the area would then immediately wrap (and vertical selection then turned off automatically). This seems to work very well ergonomically, because your hand is already on the mouse from setting the vertical/column selection itself, so it takes just a quick movement of your hand at this point to move the cursor into the selected area and click it (and the wrapping then takes effect). However, vertical/column wrapping can also be done using the keyboard (no mouse involved), so a hotkey to activate the wrapping once the area is selected (using the keyboard) would also (ideally) be required (so you don't have to grab your mouse to do it). The left arrow is a convenient hotkey in this case, since your hand is usually on the cursor keys already (to do the vertical/column selection). Whether you use the mouse or the keyboard to perform the vertical/column selection, the way you actually activate the wrapping of the selected area can be controlled via a new CodeMaid option. You'ld have one for the mouse, and one for the keyboard. where the mouse would default to a left-mouse click and the keyboard would default to the left arrow key (users can change either). Either will then wrap the vertical/column selection and turn off the selection as described. There should also (probably) be an option to enable/disable vertical/column wrapping altogether. You'll have to think all of this through of course to make sure it's all viable.

Lastly, I just want to say thanks again for your product. The wrapping feature alone has saved me an enormous amount of time, notwithstanding these potential improvements (BTW, I have an extension of my own though it's a commercial product - I won't mention the name or site however since it's not relevant here).

w5l commented 5 years ago

There is a hotkey to format the comment under the cursor, it default to CTRL + M, N but I think it can be redefined using the normal VS keyboard shortcut settings.

The only potential difficulty with custom width wrapping that I see so far is that the setting is not memorized, so a clean/format of the whole document would set all comments to the same width again. Especially when auto-format-on-save is enabled, this could cause confusion/frustration.