Day template missing % after $first_column_width [#1792368]

A missing % sign after print $first_column_width in the day template causes layout to break.

Comment	File	Size	Author
#4	calendar-day-tpl-percent-1792368-6510008.patch	1.03 KB	rdela
#4
#1	calendar-day-tpl-percent-1792368-0.patch	2.66 KB	rdela
#1
#2	calendar-day-tpl-percent-1792368-1.patch	4.75 KB	rdela
#2
	calendar-first-col.png	29.97 KB	rdela

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

rdela CreditAttribution: rdela commented 22 September 2012 at 19:44

File	Size
calendar-day-tpl-percent-1792368-0.patch	2.66 KB

patch attached

EDIT: this patch is incomplete, see reply below -
http://drupal.org/node/1792368#comment-6509856

Comment #2

rdela CreditAttribution: rdela commented 22 September 2012 at 19:38

File	Size
calendar-day-tpl-percent-1792368-1.patch	4.75 KB

also necessary to modify: calendar_multiday > calendar-day.tpl.php
in addition to: theme > calendar-day.tpl.php.

revised patch attached

Comment #3

arlinsandbulte CreditAttribution: arlinsandbulte commented 22 September 2012 at 20:45

Status:

Needs review

» Needs work

Thanks for the patch! A quick glance looks good to me.

However, this patch has a bunch of unrelated whitespace corrections.
While fixing whitespace according to standards is a good thing, I'd prefer to keep patches & commits more atomic.

Please remove the whitespace corrections from the main bugfix patch.

Thanks again!

Comment #4

rdela CreditAttribution: rdela commented 25 September 2012 at 19:12

Status:

Needs work

» Needs review

File	Size
calendar-day-tpl-percent-1792368-6510008.patch	1.03 KB

Thanks Arlin! This is my first Drupal patch ever, so I appreciate the encouragement. I noticed all the whitespace “corrections” in the diff but wasn’t sure why they were happening.

Thanks to your standards I traced the problem to the trim_trailing_white_space_on_save option I had enabled in Sublime Text 2 settings, disabled it, recreated the patch and now I think it should be good to go.

Now, not only is that former mystery no longer a source of confusion for me, but the next time I submit a patch I know to disable this before I start. I am thankful you made me resolve this because it will help any of my future patches’ chances of acceptance and save the maintainers of those modules (who might not be as friendly as you are) the trouble.

Comment #5

arlinsandbulte CreditAttribution: arlinsandbulte commented 25 September 2012 at 23:10

Status:

Needs review

» Fixed

Thanks right back at ya, rdela! Much cleaner, easier to read, and easier to tell what really is being fixed.
This issue also looked to be present in the D7 & D8 versions of Calendar, so I applied the same fix there too.

D6 commit - http://drupalcode.org/project/calendar.git/commit/eb10d95
D7 commit - http://drupalcode.org/project/calendar.git/commit/17fceb7
D8 commit - http://drupalcode.org/project/calendar.git/commit/3baf47f

Now, if you would like to fix some of the white-space issues, feel free. Patches gladly accepted.