I think the page should preserve the line breaks, it makes for easier reading.

CommentFileSizeAuthor
#6 0001-Issue-1075702-Fixed-Commits-page-does-not-preserve-l.patch1.37 KBmarvil07
#2 git-commit-log-readability-issue.png31.16 KBicc

Comments

eliza411’s picture

eliza411 commented
Status: Active » Postponed (maintainer needs more info)

Please include a link to a specific commit page where the failure to preserve line breaks is causing readability problems. If the example is likely to change over time, a screen cap would be appropriate as well.

icc’s picture

icc commented
Status: Postponed (maintainer needs more info) » Active
StatusFileSize
new git-commit-log-readability-issue.png31.16 KB

Attached a screenshot which illustrates the issue.

hass’s picture

hass commented

Same issue in piwik commit messages.

eliza411’s picture

eliza411 commented
Assigned: Unassigned » sarah_p

@sarah_p: Is this something you can take a look at?

sdboyer’s picture

sdboyer commented
Project: Git on Drupal.org » Version Control API -- Git backend
Version: » 6.x-2.x-dev
Component: User interface » Git interaction
Assigned: sarah_p » marvil07

I think this is actually a problem with how we read the data in - we read line by line, and probably destroy intentional newlines in the process. Yuck. Time to give exec() the boot in favor of proc_open()...

If I'm wrong about this, I'll switch the queues back. For the moment, though, assigning to marvil07.

marvil07’s picture

marvil07 commented
Status: Active » Needs review
StatusFileSize
new 0001-Issue-1075702-Fixed-Commits-page-does-not-preserve-l.patch1.37 KB

Thanks you very much for catching this!
Patch attached ;-)

@sdboyer: definitely we need to revisit this whole git history parser, but for now it do enough IMHO.

marvil07’s picture

marvil07 commented
Assigned: marvil07 » sdboyer

@sdboyer: This seems to be a trivial patch, so a quick review would be fine ;-)

sdboyer’s picture

sdboyer commented
Assigned: sdboyer » marvil07
Status: Needs review » Reviewed & tested by the community

wfm. push away

marvil07’s picture

marvil07 commented
Status: Reviewed & tested by the community » Fixed

Ok, it's there.

eliza411’s picture

eliza411 commented
Issue tags: +needs drupal.org deployment

Tagging for deployment

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

eliza411’s picture

eliza411 commented
Status: Closed (fixed) » Active

Not deployed yet, reopening.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Active » Fixed

It is fine to leave this fixed. It clutters up issue queues to have things that are fixed, but not deployed, hanging around. Is is okay to use -dev or do you want to roll a tagged release?

eliza411’s picture

eliza411 commented

Yeah, sorry about that ... I've figured it out a little better since April, but not all the way yet.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

marvil07’s picture

marvil07 commented
Issue tags: -needs drupal.org deployment

Deployed today

  • Commit 1cd83d2 on 6.x-2.x, fix-invalid-default-branches, fullsync-memory by marvil07: 
    Issue #1075702: Fixed Commits page does not preserve line breaks in...