From: Robert Klemme <shortcutter@...>
Date: 2007-05-14T22:55:05+09:00
Subject: Re: slow file access with mapped drives ?!

On 14.05.2007 14:56, Rebhan, Gilbert wrote:
  > -----Original Message-----
> From: Robert Klemme [mailto:shortcutter@googlemail.com] 
> Sent: Monday, May 14, 2007 2:45 PM
> To: ruby-talk ML
> Subject: Re: slow file access with mapped drives ?!
> 
> /*
> Please make sure you post the code you are actually testing - otherwise 
> everybody else will have a hard time understanding what's going on let 
> alone come up with helpful replies.
> */
> 
> i don't want to bother with bad code, but here is the whole thing,
> for you to understand what i try to achieve =

I was referring to the fact that you present one bit of code and in your 
answer to my posting you present another bit of code as if *that* was 
the original code.

However, there are quite a few things to say about this bit of code.

> ----snipp----
> 
> require 'yaml'
> require 'highline/import'
> require 'win32/registry'
> require 'find'
> 
> config=YAML.load_file("cvs_login.yaml")
> 
> cvsuser = ask("Enter CVS User:       ") {|q|
> q.default = "#{ENV["USERNAME"]}"
> q.echo = true}
> cvspass = ask("Enter password:       ") { |q| q.echo = '*' }
> puts "\n\n"
> 
> config['cvsrepos'].each {|x|
> puts "Login CVS Repository >> #{x} ..."
> IO.popen("#{config['CVSEXE']} -d
> :pserver:#{cvsuser}:#{cvspass}@cvsprod:d:/cvsrepos/#{x} login")
> }

Why do you use popen if you do not read the pipe?  *If* you use popen 
you should make sure the pipe is read from - even if you ignore what you 
find because otherwise the other process might get blocked.

> puts "Login successful !!"
> 
> def filesed!(pattern, replaceString, filename)
>   regexPattern = Regexp.compile(pattern)
>   tmpFilename = "#{filename}.#{Time.now.usec}"
>   tmpFile = File.new(tmpFilename, "w")
>     srcFile = File.open(filename)
>       srcFile.each_line do |line|
>         tmpFile.print line.gsub(regexPattern) {|match|
>         replaceString
>       }

Rather use the non block form of gsub unless you want to make sure that 
there is no metacharacter interpretation going on.  (Even in that case 
I'd probably prefer the non block form with a modified replaceString.)

>   end
>   tmpFile.close
>   srcFile .close

Rather use the block form of File.open which is much safer.

>   File.rename(tmpFilename, filename)
> end
> 
> if config['targetdirs']
> 
>   Win32::Registry::HKEY_CURRENT_USER.open('Software\Cvsnt\cvspass') do
> |reg|
>          @cvsreg =
> reg[":pserver:#{ENV["USERNAME"]}@cvsprod:d:/cvsrepos/test"]
>   end
> 
>   s=config['replaceto1']<<"#{@cvsreg}"<<config['replaceto2']

Why do you use "#{@cvsreg}" instead of plain @cvsreg or @cvsreg.to_s?

>   config['targetdirs'].each do |dir|
>     puts "\n\n"
>     Find.find(dir) do |f|
>       if f =~ /#{config['targetfilepattern']}/

You should pull out this regexp compilation from both loops for more 
efficiency.

>       puts "... processing "<<f
>       filesed!(config['replacefrom'],s,f)

Indentation.

>       end
>     end
>   end
>   puts "\n\nDone !!"
>   sleep 1

Why the sleep?

>   else
>   puts "\n\nDone !!"
>     sleep 1

Why the sleep?

>   end
> 
> ----snipp---
> 
> 
> ----yaml---
> 
> ---
> CVSEXE: "./cvsnt/cvs.exe"
> cvsrepos:
> - foo
> - foobar
> - foobaz
> - bla
> - ...
> 
> # optional stuff following
> targetdirs:
> - Y:/tempwork
> - T:/rubytest
> - //wvp10175/c$/SCM_Server
> 
> targetfilepattern: Scm\w+\.xml$
> 
> replacefrom: "<Passwort>.*</Passwort>"

This pattern will kill you if there are two sections with 
<Passwort></Passwort> in the file.  You should at least use the 
reluctanct qualifier.

> replaceto1: "<Passwort><![CDATA["
> replaceto2: "]></Passwort>"

This cries for using a regexp group.  Also filesed is pretty inflexible. 
  I'd rather do something like this:

def file_replace(file, tmp = file + Time.now.usec.to_s)
   File.open(tmp, "w") do |out|
     File.open(file) do |inf|
       inf.each_line {|line| out.puts(yield(line)) }
     end
   end
   File.mv(tmp, file, :force => true)
end

Now you can do arbitrary line based replacements like

file_replace "foo.txt" do |line|
   "# " << line
end

> ----yaml----
> 
> the filesed method is not by me, found it somewhere, don't remember
> 
> Regards, Gilbert

Regards

	robert