On 02/09/19 08:35PM, Birger Skogeng Pedersen wrote: > On Mon, Sep 2, 2019 at 8:19 PM Pratyush Yadav <me@xxxxxxxxxxxxxxxxx> wrote: > > You missed removing this extra blank line. Will fix it up locally. > > > > > + set _index $file_lists_last_clicked($widget) > > [snip] > > > To be honest I felt the blank line there was appropriate, in an > attempt to follow the same code style as the rest of the git-gui.sh. > For instance, the "add_range_to_selection" function > (https://github.com/prati0100/git-gui/blob/master/git-gui.sh#L2601-L2625) > has blank lines like this; Oh yes, these types of blank lines are perfectly fine in my opinion too. I actually prefer this alternative over not having any spacing at all. This way you divide the function in logical sections/steps. But that was not what I pointed out. The blank line I pointed out was like this: if {condition} { foo bar } My nitpick was with the blank line _just after_ the if. > > proc add_range_to_selection {w x y} { > global file_lists last_clicked selected_paths > So in this example, the above blank line is fine... > if {[lindex $last_clicked 0] ne $w} { ...but if I add a blank line here like: if {[lindex $last_clicked 0] ne $w} { toggle_or_diff click $w $x $y return } instead of like: if {[lindex $last_clicked 0] ne $w} { toggle_or_diff click $w $x $y return } It is the blank line just after the start of your if statement that I was pointing out. I did leave other blank lines untouched in your patch. [snip] -- Regards, Pratyush Yadav
