Skip to content

Various cleanups #3

Closed
wants to merge 20 commits into from

2 participants

@shlomif
shlomif commented Oct 31, 2011

Hi Rocky,

Please pull the changes here that include various cleanups like "use English qw( -no_match_vars)" , strict and warnings on separate lines, a refactoring of t/Helper.pm etc.

Note that it doesn't contain the fix for the ReadLine loading / t/20*.t tests failing on my system which is on a different branch.

Regards,

-- Shlomi Fish

shlomif added some commits Oct 26, 2011
@shlomif shlomif Made Build.PL non-executable and sha-bang-less. 959bf27
@shlomif shlomif Fix the Psh required ver to three-dots.
The Psh required version was warned when given 1.8.1 which is more
recent than "1.8" (or 1.8.0). So it was fixed here.
dbf98a7
@shlomif shlomif Remove a warning with recent perls.
Deprecated using qw(...) directly as a list.
d877a95
@shlomif shlomif Merge https://github.com/rocky/Perl-Devel-Trepan 9597cfe
@shlomif shlomif Cleaned up the use statements in t/Helper.pm.
Removed duplicate ones, those that are both on the same line, etc.
bd52240
@shlomif shlomif More loading cleanups.
Moved some use/require to the top.
b0d2ec3
@shlomif shlomif Add -no_match_vars to use English.
This makes it faster.
ee83ecb
@shlomif shlomif Add -no_match_vars to use English statements.
This is faster and more recommended (see perldoc English).
b64ddf6
@shlomif shlomif Started cleaning up t/Helper.pm. 7956382
@shlomif shlomif More Helper.pm cleanup.
Broke a line and removed an unneeded one.
a3fef16
@shlomif shlomif Extract a closure. 3dc282c
@shlomif shlomif More cleanups to Helper.pm. fdcff91
@shlomif shlomif "file" in identifiers to "fn". aa3ee3b
@shlomif shlomif Add a blurb for the is. 5afbebf
@shlomif shlomif Add an explicit return. 9abc556
@shlomif shlomif Reorganized the strict and warnings. 0e0c7bb
@shlomif shlomif More strict and warnings cleanup. 4894b10
@shlomif shlomif Separate lines for strict/warnings. 0b790fe
@shlomif shlomif use statements cleanup. 449b0f8
@shlomif shlomif Got rid of strict and warnings in the same line.
Cleaned up the loaded pragmas otherwise.
6f49cae
@rocky
Owner
rocky commented on 6f49cae Nov 1, 2011

I really think this kind of change is really silly, if not bad. I string the lines across because basically each of the lines is boilerplate and I'd like to skip over all of it as much as possible. Computer displays tend to be limited in line length more than they are in column length.

Are Perl experts so rigid that one can't tolerate different styles?

rocky:

Well, this is the first time I've seen such putting several "use" statements on the same line. It has given me a "WTF?" moment, and I found it hard to read. Furthermore, your boilerplat is a little different every time, and I also moved package statements right at the top, which is important.

If you're so keen on keeping them out of the way, you can configure your text editor to fold these lines out of the way:

http://vim.wikia.com/wiki/Folding

(This is for Vim - other decent editors should have similar features.).

Owner
rocky replied Nov 1, 2011

Is this so hard to read?

    $x = 1;  $y = 2; $z = 3; 

that you go around splitting this on several lines? And if it is

     $x = $y = $z = 0;

do you write that as:

     $x = 
     $y = 
     $z = 0;

? Or split this into 3 separate assignment statements?

Do you try to line up the = sign in:

    $first  = 1;
    $second  = 2;
    $third = 3;

?

because this is easier to read? I guess part of the point I am trying to make is that I doubt that just putting stuff on more lines in of itself makes things readable.

The thing about style is it is largely a matter of what one is used to. At one time I preferred indentation to be two spaces because again it is the minimum amount that gets the point across clearly. In some programming languages folks want 2 spaces, in some 4, and in some something different. After a while, I found I could just get used to any. So although I could get used to the multiple line thing, I think it silly and sad I can't write in the way I find the most logical and suits me the best.

When I say boilerplate, part of what I mean is that this stuff is uninteresting. Uninteresting to the point that if you totally skipped those lines and never read them, it wouldn't impact one iota the essentials of the program - the organization of classes, the algorithms, the fundamentals of what this program does and so on.

I know how to elide blocks of code, and I don't see a need to customize my Editor to elide this stuff. I want to write code in this way. The fact that you haven't seen it doesn't in of itself mean that this is a bad thing to do. Try it and live with it for a bit. If after trying it you can elucidate why it is so hard to read and why it is so important for you to be able read this, then let's discuss.

Also sad to me is that were spending this amount of time over things like this rather than:

  • adding watchpoints
  • adding out-of-process debugging
  • adding signal handlers like gdb's "handle" command
  • figuring out how to remove the globalness of the debugger to allow say profiing and debugging to be totally independent
  • finishing and removing some of the weirdnesses of tab ReadLine completion

These to me are the more important issues.

Owner
rocky replied Nov 1, 2011

One other thing I had meant to mention. You said that the boilerplate is different and that puts you off. In some cases I have "strict" before "warnings" and some the other way around. But when you split into several lines, you kept this irregularity in some cases. But again, I think this is all irrelevant; if you are spending time looking at this stuff and making sure strict comes before warnings (or vice versa) then I think you are concerned with the parts of the program that are the least important.

By different boilerplate, I did not mean "strict" before "warnings" or vice-versa (which makes little difference). Instead, I meant stuff like "no warnings $something" which are also there sometimes (and are placed on the same line).

In any case, I'm likely to run into such style issues first when starting to work on your code, and it will likely give other Perl developers besides me some "WTF?" moments. I've introduced some more meaningful changes like the "use English qw( -no_match_vars ) ;" cleanup and the t/Helper.pm cleanup, and I'm still waiting for input on why there are so many "use lib '../../../'" statements in the code (which like I said might prove to be a security breach).

@rocky
Owner
rocky commented on d877a95 Nov 4, 2011

Reine Urban later noticed this and issued a separate pull request. So this has been encorporated now.

@rocky
Owner
rocky commented on dbf98a7 Nov 4, 2011

This change has been encorporated.

@rocky
Owner
rocky commented on b0d2ec3 Nov 4, 2011

requires has been moved up. There is now a Helper package so the "use English" has to come after that.

@rocky
Owner
rocky commented on ee83ecb Nov 4, 2011

Change incorporated. Thanks.

@rocky
Owner
rocky commented on 5afbebf Nov 4, 2011

Applied with slight change.

@rocky
Owner
rocky commented on 3dc282c Nov 4, 2011

Applied.

@rocky
Owner
rocky commented on b64ddf6 Nov 4, 2011

Applied. Thanks.

@rocky
Owner
rocky commented on 9abc556 Nov 4, 2011

Applied.

@rocky
Owner
rocky commented on 4894b10 Nov 4, 2011

Most of this seems dubious.

@rocky
Owner
rocky commented on aa3ee3b Nov 4, 2011

I don't like fn since it reminds me of function. So using filename is a compromise. With this caveat, applied.

@rocky
Owner
rocky commented Nov 4, 2011

I think I have all of the important changes merged in. Some things I disagree with as noted before. If there's anything you feel missing, let me know.

Thanks for the help.

@rocky rocky closed this Nov 4, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.