From e4036cbe56de29be1a4773d459c87700884f17d0 Mon Sep 17 00:00:00 2001 From: Tuncer Ayaz Date: Sat, 8 Jan 2011 19:09:24 +0100 Subject: [PATCH] Apply Tidier suggestions --- src/getopt.erl | 22 +++++++++++----------- src/rebar_ct.erl | 2 +- src/rebar_deps.erl | 15 +++++++++------ src/rebar_dialyzer.erl | 2 +- src/rebar_erlydtl_compiler.erl | 15 +++++++-------- src/rebar_file_utils.erl | 32 ++++++++++++++++---------------- src/rebar_neotoma_compiler.erl | 19 +++++++++---------- src/rebar_port_compiler.erl | 5 ++++- src/rebar_reltool.erl | 14 ++++++++------ src/rebar_require_vsn.erl | 5 +++-- src/rebar_utils.erl | 16 ++++++++-------- test/rebar_eunit_tests.erl | 2 +- 12 files changed, 78 insertions(+), 71 deletions(-) diff --git a/src/getopt.erl b/src/getopt.erl index bb7fae2..5f9fe61 100644 --- a/src/getopt.erl +++ b/src/getopt.erl @@ -75,10 +75,10 @@ parse(OptSpecList, OptAcc, ArgAcc, _ArgPos, ["--" | Tail]) -> % Any argument present after the terminator is not considered an option. {ok, {lists:reverse(append_default_options(OptSpecList, OptAcc)), lists:reverse(ArgAcc, Tail)}}; %% Process long options. -parse(OptSpecList, OptAcc, ArgAcc, ArgPos, [[$-, $- | OptArg] = OptStr | Tail]) -> +parse(OptSpecList, OptAcc, ArgAcc, ArgPos, ["--" ++ OptArg = OptStr | Tail]) -> parse_option_long(OptSpecList, OptAcc, ArgAcc, ArgPos, Tail, OptStr, OptArg); %% Process short options. -parse(OptSpecList, OptAcc, ArgAcc, ArgPos, [[$- | [_Char | _] = OptArg] = OptStr | Tail]) -> +parse(OptSpecList, OptAcc, ArgAcc, ArgPos, ["-" ++ ([_Char | _] = OptArg) = OptStr | Tail]) -> parse_option_short(OptSpecList, OptAcc, ArgAcc, ArgPos, Tail, OptStr, OptArg); %% Process non-option arguments. parse(OptSpecList, OptAcc, ArgAcc, ArgPos, [Arg | Tail]) -> @@ -111,11 +111,11 @@ parse_option_long(OptSpecList, OptAcc, ArgAcc, ArgPos, Args, OptStr, OptArg) -> parse_option_assigned_arg(OptSpecList, OptAcc, ArgAcc, ArgPos, Args, OptStr, Long, Arg); Long -> - case lists:keysearch(Long, ?OPT_LONG, OptSpecList) of - {value, {Name, _Short, Long, undefined, _Help}} -> + case lists:keyfind(Long, ?OPT_LONG, OptSpecList) of + {Name, _Short, Long, undefined, _Help} -> parse(OptSpecList, [Name | OptAcc], ArgAcc, ArgPos, Args); - {value, {_Name, _Short, Long, _ArgSpec, _Help} = OptSpec} -> + {_Name, _Short, Long, _ArgSpec, _Help} = OptSpec -> % The option argument string is empty, but the option requires % an argument, so we look into the next string in the list. parse_option_next_arg(OptSpecList, OptAcc, ArgAcc, ArgPos, Args, OptSpec); @@ -132,8 +132,8 @@ parse_option_long(OptSpecList, OptAcc, ArgAcc, ArgPos, Args, OptStr, OptArg) -> [string()], string(), string(), string()) -> {ok, {[option()], [string()]}}. parse_option_assigned_arg(OptSpecList, OptAcc, ArgAcc, ArgPos, Args, OptStr, Long, Arg) -> - case lists:keysearch(Long, ?OPT_LONG, OptSpecList) of - {value, {_Name, _Short, Long, ArgSpec, _Help} = OptSpec} -> + case lists:keyfind(Long, ?OPT_LONG, OptSpecList) of + {_Name, _Short, Long, ArgSpec, _Help} = OptSpec -> case ArgSpec of undefined -> throw({error, {invalid_option_arg, OptStr}}); @@ -151,7 +151,7 @@ parse_option_assigned_arg(OptSpecList, OptAcc, ArgAcc, ArgPos, Args, OptStr, Lon split_assigned_arg(OptStr) -> split_assigned_arg(OptStr, OptStr, []). -split_assigned_arg(_OptStr, [$= | Tail], Acc) -> +split_assigned_arg(_OptStr, "=" ++ Tail, Acc) -> {lists:reverse(Acc), Tail}; split_assigned_arg(OptStr, [Char | Tail], Acc) -> split_assigned_arg(OptStr, Tail, [Char | Acc]); @@ -170,11 +170,11 @@ split_assigned_arg(OptStr, [], _Acc) -> -spec parse_option_short([option_spec()], [option()], [string()], integer(), [string()], string(), string()) -> {ok, {[option()], [string()]}}. parse_option_short(OptSpecList, OptAcc, ArgAcc, ArgPos, Args, OptStr, [Short | Arg]) -> - case lists:keysearch(Short, ?OPT_SHORT, OptSpecList) of - {value, {Name, Short, _Long, undefined, _Help}} -> + case lists:keyfind(Short, ?OPT_SHORT, OptSpecList) of + {Name, Short, _Long, undefined, _Help} -> parse_option_short(OptSpecList, [Name | OptAcc], ArgAcc, ArgPos, Args, OptStr, Arg); - {value, {_Name, Short, _Long, ArgSpec, _Help} = OptSpec} -> + {_Name, Short, _Long, ArgSpec, _Help} = OptSpec -> case Arg of [] -> % The option argument string is empty, but the option requires diff --git a/src/rebar_ct.erl b/src/rebar_ct.erl index d6d4675..0c8e2ef 100644 --- a/src/rebar_ct.erl +++ b/src/rebar_ct.erl @@ -137,7 +137,7 @@ make_cmd(TestDir, Config) -> %% that are part of the root Erlang install are filtered out to %% avoid duplication R = code:root_dir(), - NonLibCodeDirs = [P || P <- code:get_path(), lists:prefix(R, P) == false], + NonLibCodeDirs = [P || P <- code:get_path(), not lists:prefix(R, P)], CodeDirs = [io_lib:format("\"~s\"", [Dir]) || Dir <- [EbinDir|NonLibCodeDirs]], CodePathString = string:join(CodeDirs, " "), diff --git a/src/rebar_deps.erl b/src/rebar_deps.erl index 72d982a..d93b158 100644 --- a/src/rebar_deps.erl +++ b/src/rebar_deps.erl @@ -134,8 +134,9 @@ compile(Config, AppFile) -> DepsDir = get_deps_dir(), Deps = rebar_config:get_local(Config, deps, []), {AvailableDeps, _} = find_deps(find, Deps), - _ = [delete_dep(D) || D <- AvailableDeps, - lists:prefix(DepsDir, D#dep.dir) == true], + _ = [delete_dep(D) + || D <- AvailableDeps, + lists:prefix(DepsDir, D#dep.dir)], ok. @@ -339,11 +340,13 @@ update_source(Dep) -> end. update_source(AppDir, {git, _Url, {branch, Branch}}) -> - rebar_utils:sh("git fetch origin", [{cd, AppDir}]), - rebar_utils:sh(?FMT("git checkout -q origin/~s", [Branch]), [{cd, AppDir}]); + ShOpts = [{cd, AppDir}], + rebar_utils:sh("git fetch origin", ShOpts), + rebar_utils:sh(?FMT("git checkout -q origin/~s", [Branch]), ShOpts); update_source(AppDir, {git, _Url, {tag, Tag}}) -> - rebar_utils:sh("git fetch --tags origin", [{cd, AppDir}]), - rebar_utils:sh(?FMT("git checkout -q ~s", [Tag]), [{cd, AppDir}]); + ShOpts = [{cd, AppDir}], + rebar_utils:sh("git fetch --tags origin", ShOpts), + rebar_utils:sh(?FMT("git checkout -q ~s", [Tag]), ShOpts); update_source(AppDir, {git, Url, Refspec}) -> update_source(AppDir, {git, Url, {branch, Refspec}}); update_source(AppDir, {svn, _Url, Rev}) -> diff --git a/src/rebar_dialyzer.erl b/src/rebar_dialyzer.erl index 9d45445..acaf69a 100644 --- a/src/rebar_dialyzer.erl +++ b/src/rebar_dialyzer.erl @@ -193,7 +193,7 @@ existing_plt_path(Config, File) -> ?ABORT("No PLT found~n", []) end end; - [$~|[$/|Plt]] -> + "~/" ++ Plt -> filename:join(Home,Plt); Plt -> Plt diff --git a/src/rebar_erlydtl_compiler.erl b/src/rebar_erlydtl_compiler.erl index b746642..d30b56c 100644 --- a/src/rebar_erlydtl_compiler.erl +++ b/src/rebar_erlydtl_compiler.erl @@ -164,14 +164,13 @@ referenced_dtls1(Step, Config, Seen) -> [{return, list}]), AllRefs = lists:append( - lists:map( - fun(F) -> - {ok, Res} = rebar_utils:sh( - lists:flatten(["grep -o [^\\\"]*", - ExtMatch," ",F]), - [{use_stdout, false}]), - string:tokens(Res, "\n") - end, Step)), + [begin + {ok, Res} = rebar_utils:sh( + lists:flatten(["grep -o [^\\\"]*", + ExtMatch," ",F]), + [{use_stdout, false}]), + string:tokens(Res, "\n") + end || F <- Step]), DocRoot = option(doc_root, DtlOpts), WithPaths = [ filename:join([DocRoot, F]) || F <- AllRefs ], Existing = [F || F <- WithPaths, filelib:is_regular(F)], diff --git a/src/rebar_file_utils.erl b/src/rebar_file_utils.erl index 0a7cd4c..026b06b 100644 --- a/src/rebar_file_utils.erl +++ b/src/rebar_file_utils.erl @@ -49,8 +49,8 @@ rm_rf(Target) -> ok; {win32, _} -> Filelist = filelib:wildcard(Target), - Dirs = lists:filter(fun filelib:is_dir/1,Filelist), - Files = lists:subtract(Filelist,Dirs), + Dirs = [F || F <- Filelist, filelib:is_dir(F)], + Files = Filelist -- Dirs, ok = delete_each(Files), ok = delete_each_dir_win32(Dirs), ok @@ -82,9 +82,10 @@ mv(Source, Dest) -> [filename:nativename(Source), filename:nativename(Dest)]), [{use_stdout, false}, return_on_error]), - case length(R) == 0 of - true -> ok; - false -> + case R of + [] -> + ok; + _ -> {error, lists:flatten( io_lib:format("Failed to move ~s to ~s~n", [Source, Dest]))} @@ -130,25 +131,24 @@ xcopy_win32(Source,Dest)-> [Source, Dest]))} end. -cp_r_win32({true,SourceDir},{true,DestDir}) -> +cp_r_win32({true, SourceDir}, {true, DestDir}) -> % from directory to directory SourceBase = filename:basename(SourceDir), - ok = case file:make_dir(filename:join(DestDir,SourceBase)) of - {error,eexist} -> ok; + ok = case file:make_dir(filename:join(DestDir, SourceBase)) of + {error, eexist} -> ok; Other -> Other end, - ok = xcopy_win32(SourceDir,filename:join(DestDir,SourceBase)); -cp_r_win32({false,Source},{true,DestDir}) -> + ok = xcopy_win32(SourceDir, filename:join(DestDir, SourceBase)); +cp_r_win32({false, Source} = S,{true, DestDir}) -> % from file to directory - cp_r_win32({false,Source}, - {false,filename:join(DestDir,filename:basename(Source))}); -cp_r_win32({false,Source},{false,Dest}) -> + cp_r_win32(S, {false, filename:join(DestDir, filename:basename(Source))}); +cp_r_win32({false, Source},{false, Dest}) -> % from file to file - {ok,_} = file:copy(Source,Dest), + {ok,_} = file:copy(Source, Dest), ok; cp_r_win32(Source,Dest) -> - Dst = {filelib:is_dir(Dest),Dest}, + Dst = {filelib:is_dir(Dest), Dest}, lists:foreach(fun(Src) -> - ok = cp_r_win32({filelib:is_dir(Src),Src},Dst) + ok = cp_r_win32({filelib:is_dir(Src), Src}, Dst) end, filelib:wildcard(Source)), ok. diff --git a/src/rebar_neotoma_compiler.erl b/src/rebar_neotoma_compiler.erl index ce7a0d3..e67007c 100644 --- a/src/rebar_neotoma_compiler.erl +++ b/src/rebar_neotoma_compiler.erl @@ -121,16 +121,15 @@ referenced_pegs1(Step, Config, Seen) -> NeoOpts = neotoma_opts(Config), ExtMatch = re:replace(option(source_ext, NeoOpts), "\.", "\\\\\\\\.", [{return, list}]), - AllRefs = - lists:append( - lists:map( - fun(F) -> - {ok, Res} = rebar_utils:sh( - lists:flatten(["grep -o [^\\\"]*", - ExtMatch," ",F]), - [{use_stdout, false}]), - string:tokens(Res, "\n") - end, Step)), + + AllRefs = lists:append([begin + {ok, Res} = + rebar_utils:sh( + lists:flatten(["grep -o [^\\\"]*", + ExtMatch, " ", F]), + [{use_stdout, false}]), + string:tokens(Res, "\n") + end || F <- Step]), DocRoot = option(doc_root, NeoOpts), WithPaths = [ filename:join([DocRoot, F]) || F <- AllRefs ], Existing = [F || F <- WithPaths, filelib:is_regular(F)], diff --git a/src/rebar_port_compiler.erl b/src/rebar_port_compiler.erl index c1a6500..3bcbb26 100644 --- a/src/rebar_port_compiler.erl +++ b/src/rebar_port_compiler.erl @@ -119,7 +119,10 @@ clean(Config, AppFile) -> rebar_file_utils:delete_each([source_to_bin(S) || S <- Sources]), %% Delete the .so file - rebar_file_utils:delete_each(lists:map(fun({SoName,_}) -> SoName end, so_specs(Config, AppFile, expand_objects(Sources)))), + ExtractSoName = fun({SoName, _}) -> SoName end, + rebar_file_utils:delete_each([ExtractSoName(S) + || S <- so_specs(Config, AppFile, + expand_objects(Sources))]), %% Run the cleanup script, if it exists run_cleanup_hook(Config). diff --git a/src/rebar_reltool.erl b/src/rebar_reltool.erl index b8e1095..d375e97 100644 --- a/src/rebar_reltool.erl +++ b/src/rebar_reltool.erl @@ -222,13 +222,15 @@ run_reltool(Server, _Config, ReltoolConfig) -> end, %% Finally, overlay the files specified by the overlay section - case lists:keysearch(overlay, 1, ReltoolConfig) of - {value, {overlay, Overlay}} when is_list(Overlay) -> - execute_overlay(Overlay, OverlayVars, rebar_utils:get_cwd(), TargetDir); - {value, _} -> - ?ABORT("{overlay, [...]} entry in reltool.config must be a list.\n", []); + case lists:keyfind(overlay, 1, ReltoolConfig) of + {overlay, Overlay} when is_list(Overlay) -> + execute_overlay(Overlay, OverlayVars, rebar_utils:get_cwd(), + TargetDir); false -> - ?INFO("No {overlay, [...]} found in reltool.config.\n", []) + ?INFO("No {overlay, [...]} found in reltool.config.\n", []); + _ -> + ?ABORT("{overlay, [...]} entry in reltool.config " + "must be a list.\n", []) end; {error, Reason} -> diff --git a/src/rebar_require_vsn.erl b/src/rebar_require_vsn.erl index 96fc088..be51be9 100644 --- a/src/rebar_require_vsn.erl +++ b/src/rebar_require_vsn.erl @@ -45,7 +45,8 @@ eunit(Config, _) -> check_versions(Config) -> ErtsRegex = rebar_config:get(Config, require_erts_vsn, ".*"), - case re:run(erlang:system_info(version), ErtsRegex, [{capture, none}]) of + ReOpts = [{capture, none}], + case re:run(erlang:system_info(version), ErtsRegex, ReOpts) of match -> ?DEBUG("Matched required ERTS version: ~s -> ~s\n", [erlang:system_info(version), ErtsRegex]); @@ -55,7 +56,7 @@ check_versions(Config) -> end, OtpRegex = rebar_config:get(Config, require_otp_vsn, ".*"), - case re:run(erlang:system_info(otp_release), OtpRegex, [{capture, none}]) of + case re:run(erlang:system_info(otp_release), OtpRegex, ReOpts) of match -> ?DEBUG("Matched required OTP release: ~s -> ~s\n", [erlang:system_info(otp_release), OtpRegex]); diff --git a/src/rebar_utils.erl b/src/rebar_utils.erl index f49969a..a9ed9a0 100644 --- a/src/rebar_utils.erl +++ b/src/rebar_utils.erl @@ -75,8 +75,8 @@ sh(Command0, Options0) -> ?INFO("sh: ~s\n~p\n", [Command0, Options0]), DefaultOptions = [use_stdout, abort_on_error], - Options = lists:map(fun expand_sh_flag/1, - proplists:compact(Options0 ++ DefaultOptions)), + Options = [expand_sh_flag(V) + || V <- proplists:compact(Options0 ++ DefaultOptions)], ErrorHandler = proplists:get_value(error_handler, Options), OutputHandler = proplists:get_value(output_handler, Options), @@ -87,8 +87,8 @@ sh(Command0, Options0) -> Port = open_port({spawn, Command}, PortSettings), case sh_loop(Port, OutputHandler, []) of - {ok, Output} -> - {ok, Output}; + {ok, _Output} = Ok -> + Ok; {error, Rc} -> ErrorHandler(Command, Rc) end. @@ -179,10 +179,10 @@ expand_sh_flag({use_stdout, false}) -> fun(Line, Acc) -> [Acc | Line] end}; -expand_sh_flag({cd, Dir}) -> - {port_settings, {cd, Dir}}; -expand_sh_flag({env, Env}) -> - {port_settings, {env, Env}}. +expand_sh_flag({cd, _CdArg} = Cd) -> + {port_settings, Cd}; +expand_sh_flag({env, _EnvArg} = Env) -> + {port_settings, Env}. -spec log_and_abort(string(), integer()) -> no_return(). log_and_abort(Command, Rc) -> diff --git a/test/rebar_eunit_tests.erl b/test/rebar_eunit_tests.erl index 70e5d33..4179c9b 100644 --- a/test/rebar_eunit_tests.erl +++ b/test/rebar_eunit_tests.erl @@ -252,5 +252,5 @@ assert_full_coverage(Mod) -> string:str(X, Mod) =/= 0, string:str(X, "100%") =/= 0], file:close(F), - ?assert(length(Result) == 1) + ?assert(length(Result) =:= 1) end.