Code review comment for lp:~csurbhi/ubuntu/natty/plymouth/plymouth.bug-fix566818

Hi Surbhi,

+ * Returns the position of the last newline character.
+ * If the newline character is at the end of the string,
+ * it returns the previous newline character in the string
+ * if any.

Why backing up to the previous newline character? This seems to just result in additional special handling down the line when write_on_views() is called, and is inconsistent anyway with the current handling as it means the prompt will lose the final newline, and the bullets will be drawn on the same line as the prompt instead of on the line after it. (Do you have examples of such a prompt today that ends in a newline?) It wouldn't be good form to end the prompt with a newline, but I don't think the details plugin is the place to correct for this.

+ write_on_views (plugin,
+ prompt,
+ prompt[len-1] == '\n' ? len -1 : len);
+ } else
+ write_on_views (plugin,
+ prompt,
+ strlen (prompt));
+ }

If final newlines don't need special treatment (and I think they shouldn't), then we can reduce the code duplication here as well.

   else
     write_on_views (plugin,
- "Password",
- strlen ("Password"));
-
- write_on_views (plugin, ":", strlen (":"));
+ "Password:",
+ strlen ("Password:"));

This is a fix for the separate issue of a doubled ':' in the prompt, right? It would be best if this were called out separately - perhaps in a dep3 header in debian/patches/ubuntu_details.patch that explains what the patch is for. (No need to split this as a separate commit or go into great detail in the changelog, but it should be documented somewhere so that it's clear this isn't an accidental change.)