Added the ability to use \n and/or \r in HelpText

[ericnewton76]

Issue by rupe120
Monday Jun 22, 2015 at 19:41 GMT
Originally opened as gsscoder/commandline#169

---

It would be helpful to be able to create more than one paragraph in the HelpText, for the instances where there is supplemental / conditional information to add.

I have a scenario where depending on the list of options supplied, a given option may have slightly different behavior and/or be required or not.

It tends to be helpful if the user can see clear separation between the default functionality of an option, and additional scenarios.

---

rupe120 included the following code: https://github.com/gsscoder/commandline/pull/169/commits

[ericnewton76]

Comment by gsscoder
Tuesday Jun 23, 2015 at 04:12 GMT

---

I've added comments to the PR #168 that you closed... This is the new version of the PR?
If yes, can I ask you to add a test relative to changes?

[ericnewton76]

Comment by rupe120
Tuesday Jun 23, 2015 at 12:22 GMT

---

Yes this is a new version, I apologize for the confusion and I will add a test

[ericnewton76]

Comment by gsscoder
Tuesday Jun 23, 2015 at 13:16 GMT

---

@rupe120, no problem... 👍 If you need to ask something, don't hesitate.

[ericnewton76]

Comment by rupe120
Tuesday Jun 23, 2015 at 17:08 GMT

---

Thanks! Will do

On Tue, Jun 23, 2015 at 9:16 AM, Giacomo Stelluti Scala <
[email protected]> wrote:

@rupe120 https://github.com/rupe120, no problem... [image: 👍] If you
need to ask something, don't hesitate.

—
Reply to this email directly or view it on GitHub
gsscoder/commandline#169 (comment).

[ericnewton76]

Comment by gsscoder
Tuesday Jun 23, 2015 at 18:15 GMT

---

So it's OK, I wait you test addition and if possible (and you've a time) a sample (side by side) of current help output and what you want to get.

Have a nice day!

[ericnewton76]

Comment by rupe120
Tuesday Jun 23, 2015 at 18:26 GMT

---

Np, the sample should be fairly easy

On Tue, Jun 23, 2015 at 2:15 PM, Giacomo Stelluti Scala <
[email protected]> wrote:

So it's OK, I wait you test addition and if possible (and you've a time) a
sample (side by side) of current help output and what you want to get.

Have a nice day!

—
Reply to this email directly or view it on GitHub
gsscoder/commandline#169 (comment).

[ericnewton76]

Comment by rupe120
Tuesday Jun 23, 2015 at 19:17 GMT

---

Hey guys, heads up on xunit. They've changed their runner and switched from an extension to a NuGet package http://xunit.github.io/docs/running-tests-in-vs.html

[ericnewton76]

Comment by rupe120
Tuesday Jun 23, 2015 at 19:29 GMT

---

The before and after

[ericnewton76]

Comment by rupe120
Tuesday Jun 23, 2015 at 19:31 GMT

---

btw, I actually removed \r from the processing because I thought it would be confusing to see \r\n and actually receive 2 lines. While I did want \n\n to give you 2 lines, the distinction would be more logic then I felt necessary. So I just eliminated the \r

[ericnewton76]

Comment by indigotock
Tuesday Jun 23, 2015 at 23:49 GMT

---

Would using Environment.NewLine not be better?

[ericnewton76]

Comment by gsscoder
Wednesday Jun 24, 2015 at 03:51 GMT

---

@indigotock, yes, is also consistent with the rest of HelpText class. @rupe120, can you do this last change to your PR, please?

(If you've done in it, please forgive me, have you verified if after the change all tests are green?)

Ping me when you're done, I'll fetch the PR locally to proceed.

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 12:31 GMT

---

Ok so, I agree that we need to keep everything consistent. On the other hand, I'm seeing this context as being different from the other places where Environment.NewLine is used. For all other places in the code the Environment.NewLine reference is used in the background to create the string that will be fed to the console. In this instance our interface is between the developer in what they have typed for help text and the help text processing, instead of the help text processing and the console.

If I use the Environment.NewLine instead of a raw \n then the users will need to use Environment.NewLine in their help text strings as well. It seems to me that using \n will be an easier interface for the developer to use.

Thoughts?

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 12:38 GMT

---

With that said, I agree that there should be a comment next to the .split() call that uses \n explaining why the break in pattern because anyone else looking at the code will have the same question.

[ericnewton76]

Comment by indigotock
Wednesday Jun 24, 2015 at 12:38 GMT

---

I think that users implementing new lines "incorrectly" is something which shouldn't be counted on by the library? Quite interested to hear other peoples' opinions on this. Perhaps there could be an enum flag which allows the author to specify the new line sequence? Something like

enum NewLineStyle{
    LineFeed,
    CarriageReturnLineFeed,
    EnvironmentNewLine,
    Any
}

Although this feel a little bit over-engineered

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 12:42 GMT

---

Though to do this "correctly" would make for more awkward HelpText creation, either using String.Format() or concatenation

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 12:47 GMT

---

I think it's more developer friendly to view attribute property text as a version of formatted text, to minimize the logic they need to surround the string with

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 12:49 GMT

---

But that's just me :o)

[ericnewton76]

Comment by Mizipzor
Wednesday Jun 24, 2015 at 12:51 GMT

---

Usually when I tackle the issue of platform dependent new line characters I choose one to use internally, usually \n, then everywhere input is read and Environment.NewLine is not equal to \n I coerce. And in the same way everywhere output is written to Console I also coerce.

Here we have a bit of a corner case because a user of the library would supply text with newlines that shouldn't (imo) be coerced and has to know to stick to \n or whatever internal representation we decide upon.

@rupe120 Please edit your existing post if you feel you need to amend something, you created 3 posts in 6 minutes.

[ericnewton76]

Comment by indigotock
Wednesday Jun 24, 2015 at 12:53 GMT

---

It's just occurred to me that C# can split strings with arrays of strings, not just chars (not sure on the performance of this):

Split(new string[] { "\r\n", "\n", Environment.NewLine }, StringSplitOptions.Whatever? )

Failing that, one could replace all occurances of Environment.NewLine with \n before processing, and keep splitting on \n

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 13:01 GMT

---

@mizipzor My apologies for the comment spam. Will do.

@indigotock I can do that, and I think that might actually lead to more expected and intuitive behavior. In addition, I also believe the array argument is the default operation of split so it should be just as performant, that and we aren't really dealing with massive amounts of text here.

I vote for @indigotock's string array suggestion

[ericnewton76]

Comment by indigotock
Wednesday Jun 24, 2015 at 13:06 GMT

---

An issue with that is I'm now sure how it will handle splitting on \n when \r\n is in the string, I'm assuming it iterates over the array in order but you'd have to test it

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 13:15 GMT

---

I'll test it, but I believe if you have \r\n before \n it will work as expected

[ericnewton76]

Comment by Mizipzor
Wednesday Jun 24, 2015 at 14:32 GMT

---

Spotted another issue, assuming we are trying to get this merged, the file CommandLine.sln.DotSettings.user shouldn't be commited. That is ReSharper settings specific for you and you alone. The corresponding file for shared settings is called CommandLine.sln.DotSettings and that can be (and should be) commited. It doesn't exist in this repository right now, I added it as part of my cleanup branch.

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 15:27 GMT

---

I'm not sure I grock exactly what's going on here but the end result is, the order doesn't matter for the different new line styles in the array argument for the split method

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 15:58 GMT

---

Let me know how that works.

Also note that I added a .gitignore line to exclude *.DotSettings.user

[ericnewton76]

Comment by Mizipzor
Wednesday Jun 24, 2015 at 17:06 GMT

---

While you added an ignore for the file the file itself is still commited. :)

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 17:15 GMT

---

Ya, it was to metaphorically stop the bleeding. How do you remove a file from a repo?

Is the only way to:

• undo my .gitignore update
• remove the file from the project scope
• commit the fact that it's no longer there
• re-do the .gitignore setting

[ericnewton76]

Comment by Mizipzor
Wednesday Jun 24, 2015 at 18:04 GMT

---

Yes, that is the simple and obvious way to do it. Though this is a good opportunity to learn some advanced Github. If you want, here's what I would do. Note that all this happens locally, everything currently online serves as a backup, so go nuts.

Edit: It's important to have the branch you want to rebase checked out:
git checkout master

git rebase -i 90291d4^ or whatever the hash of the commit we want to change

You now get a document in your default text editor. Here's how my look (I fetched your branch):

pick 90291d4 added test with line feeds
pick 68252d1 added the ability to use either Unix or Windows new lines in help text
pick a7005ad removed DotSettings.user files
pick 45dec68 re-added the gitignore for DotSettings.user

We want to change this into the following:

edit 90291d4 added test with line feeds
pick 68252d1 added the ability to use either Unix or Windows new lines in help text
pick 45dec68 re-added the gitignore for DotSettings.user

Note the word edit, we are going to change that commit. And one commit has been removed entirely. Save and close the document.

In the console now you should see that Git is waiting for you to edit the relevant commit. Open the folder and delete the .user file(s). Then:

git commit --all --amend --no-edit
git rebase --continue

Git will now complain about a commit being empty since we've changed stuff around. Do:

git reset
get rebase --continue

That should be it. 😃 At this point look over your commits. Up until now all has been locally, until you push this everything online serves as a backup. To force push this replacing the current commits in the branch:

git push --force

Here's some extra documentation if you like reading. As an example and making sure it works I did this locally and pushed as a branch on my fork. An interesting tidbit here that I like is that we are both listed on the commit, you're the author and I'm commiter, since I've changed it. Even if I would do cleanup, you still get credit.

You now know more Git. Gratz.

[ericnewton76]

Comment by gsscoder
Wednesday Jun 24, 2015 at 19:20 GMT

---

@mizipzor, this is useful to me too! 👍

[ericnewton76]

Comment by rupe120
Wednesday Jun 24, 2015 at 19:26 GMT

---

@mizipzor These are some awesome Git ninja moves! I think I'm going to need to read through a couple times to really get my head around it. Thank you so much!