Trim commit description from SSL

[alex-statsig]

Trim commit description from SSL

Currently, when a commit is created with a title/description, the PR/commit description is:

{title}

{desc}

When the description was parsed out previously, the first line (and trailing linebreak) were stripped, but the line in between title/desc was left around. This meant every time you amended a commit/PR, another linebreak got added to the top.

I figure its probably fine to just trim whitespace from the description in general, but could update this to only trim leading whitespace (cant really just change the slice() index to + 2 because the commit may come from another source than sapling, and may not contain this extra linebreak)

[facebook-github-bot]

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[quark-zju]

The commit in JSON format is intended to provide the raw commit information. It seems nice to be consistent with log -r . -T json's "desc" field, which preserves spaces (we might want to migrate the isl-server implementation to a log -Tjson-like eventually).

It sounds like this might be also fixed in the presentation layer. Do you have a specific React component that you'd like to change?

[alex-statsig]

The commit in JSON format is intended to provide the raw commit information. It seems nice to be consistent with log -r . -T json's "desc" field, which preserves spaces (we might want to migrate the isl-server implementation to a log -Tjson-like eventually).

It sounds like this might be also fixed in the presentation layer. Do you have a specific React component that you'd like to change?

Perhaps instead we should set the commit's description (from ISL amends) as:

{title}
{desc}

rather than

{title}

{desc}

The issue is that the mapping from "commit metadata -> isl commit state (for UI)" and "isl commit state (from UI) -> commit" is not a correct inverse function (one direction adds a line break in between title/desc in the PR body, and the other does not remove the line break). I attempted to fix this by updating "commit metadata -> isl commit state" to remove the linebreak, but perhaps we could fix the other way instead (I assumed we wanted to keep that extra linebreak for better rendering in PR body or something)

I don't think fixing at the ISL presentation layer makes a ton of sense given its an encode/decode issue, but I'd be okay with that if it makes more sense to you.

You can reproduce the issue by repeatedly amending the description of a commit in ISL; each amend will add another line break to the top (it first optimistically includes two extra line breaks interestingly, but once the actual response from sl ssl comes back, the single line break appears)

[quark-zju]

I see. The ISL description does not include the title (while the CLI {desc} template means full commit message from help template). It seems your change is fine in this case.

I think we'd like a "raw commit message" field in ISL and make the "title" and "description" as derived from the raw message. But that can be changed in the future.

I'll try to land your change after tests. Thanks!

[facebook-github-bot]

This pull request has been merged in 1eac0cf.