Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Format range] Add --range as a command line option. #299

Merged
merged 4 commits into from
Jun 4, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions src/erlfmt.erl
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,20 @@
main/1,
init/1,
format_file/2,
format_file_range/4,
format_string/2,
format_string_range/4,
format_nodes/2,
read_nodes/1,
read_nodes_string/2,
format_error/1,
format_error_info/1
]).

% For unit tests
-export([
format_file_range/4,
format_string_range/4
]).

-export_type([error_info/0, config/0, pragma/0]).

-type error_info() :: {file:name_all(), erl_anno:location(), module(), Reason :: any()}.
Expand Down Expand Up @@ -62,6 +66,18 @@ init(State) ->
-spec format_file(file:name_all() | stdin, config()) ->
{ok, [unicode:chardata()], [error_info()]} | {skip, string()} | {error, error_info()}.
format_file(FileName, Options) ->
Range = proplists:get_value(range, Options),
case Range of
undefined ->
% Whole file (default).
format_file_full(FileName, Options);
{Start, End} ->
format_file_range(FileName, {Start, 1}, {End, ?DEFAULT_WIDTH}, Options)
end.

-spec format_file_full(file:name_all() | stdin, config()) ->
{ok, [unicode:chardata()], [error_info()]} | {skip, string()} | {error, error_info()}.
format_file_full(FileName, Options) ->
PrintWidth = proplists:get_value(print_width, Options, ?DEFAULT_WIDTH),
Pragma = proplists:get_value(pragma, Options, ignore),
try
Expand Down Expand Up @@ -91,6 +107,21 @@ format_file(FileName, Options) ->
-spec format_string(string(), config()) ->
{ok, string(), [error_info()]} | {skip, string()} | {error, error_info()}.
format_string(String, Options) ->
Range = proplists:get_value(range, Options),
case Range of
undefined ->
% Whole file (default).
format_string_full(String, Options);
slaykachu marked this conversation as resolved.
Show resolved Hide resolved
{Start, End} ->
% Remove 'range' property: when applicable we pass explictly the range instead.
% Also, match specifition of format_string_range.
Options2 = proplists:remove(range, Options),
format_string_range(String, {Start, 1}, {End, ?DEFAULT_WIDTH}, Options2)
end.

-spec format_string_full(string(), config()) ->
{ok, string(), [error_info()]} | {skip, string()} | {error, error_info()}.
format_string_full(String, Options) ->
PrintWidth = proplists:get_value(print_width, Options, ?DEFAULT_WIDTH),
Pragma = proplists:get_value(pragma, Options, ignore),
try
Expand Down
58 changes: 49 additions & 9 deletions src/erlfmt_cli.erl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
verbose = false :: boolean(),
print_width = undefined :: undefined | pos_integer(),
pragma = ignore :: erlfmt:pragma(),
out = standard_out :: out()
out = standard_out :: out(),
range = undefined :: undefined | {pos_integer(), pos_integer()}
}).

-type parsed() :: {format, list(), list(), #config{}} | help | version | {error, string()}.
Expand Down Expand Up @@ -50,6 +51,11 @@ opts() ->
{delete_pragma, undefined, "delete-pragma", undefined,
"Deletes the @format pragma at the top of formatted files. "
"It will also reformat the file, but is only applied to files with a pragma, see --require-pragma."},
% The getopt module doesn't support tuple of integers as a type,
% So we accept a string, and do an ad-hoc parsing manually.
{range, undefined, "range", string,
"Range to be formatted in start-end format (both inclusive). "
"Warning! A bigger range might end up being formatted, as for now the format granularity is top-level form."},
{exclude_files, undefined, "exclude-files", string,
"files not to format. "
"This overrides the files specified to format"},
Expand Down Expand Up @@ -156,21 +162,27 @@ unprotected_with_config(Name, ParsedConfig) ->
end.

format_file(FileName, Config) ->
#config{pragma = Pragma, print_width = PrintWidth, verbose = Verbose, out = Out} = Config,
#config{pragma = Pragma, print_width = PrintWidth, verbose = Verbose, out = Out, range = Range} =
Config,
case Verbose of
true -> io:format(standard_error, "Formatting ~s\n", [FileName]);
false -> ok
end,
Options =
[{pragma, Pragma}] ++
[{print_width, PrintWidth} || PrintWidth =/= undefined] ++
[verbose || Verbose],
[verbose || Verbose] ++
[{range, Range} || range =/= undefined],
slaykachu marked this conversation as resolved.
Show resolved Hide resolved
Result =
case {Out, FileName} of
{check, stdin} ->
case {Range, Out, FileName} of
{undefined, check, stdin} ->
check_stdin(Options);
{check, _} ->
{undefined, check, _} ->
check_file(FileName, Options);
{{_, _}, check, _} ->
{error, "Checking of range not supported."};
{{_, _}, replace, _} ->
{error, "In place formatting of range not supported yet."};
_ ->
erlfmt:format_file(FileName, Options)
end,
Expand Down Expand Up @@ -311,6 +323,28 @@ parse_opts([delete_pragma | _Rest], _Files, _Exclude, #config{pragma = require})
{error, "Cannot use both --require-pragma and --delete-pragma options together."};
parse_opts([delete_pragma | Rest], Files, Exclude, Config) ->
parse_opts(Rest, Files, Exclude, Config#config{pragma = delete});
parse_opts([{range, String} | Rest], Files, Exclude, Config) ->
% Ad-hoc parsing. Mitigation for the absence of "couple of integers" direct support.
Range1 = string:split(String, ",", all),
Range2 = [list_to_integer(X) || X <- Range1],
Range3 =
case Range2 of
[X] ->
slaykachu marked this conversation as resolved.
Show resolved Hide resolved
% Single line. Set end=start
[X, X];
[X, Y] ->
[X, Y];
_ ->
{error, "Range: Expected 1 argument (single line) or 2 (start, end)."}
end,
case Range3 of
[Start, End] when Start > End ->
{error, "Range: End must be greater or equal than Start."};
[Start, End] ->
parse_opts(Rest, Files, Exclude, Config#config{range = {Start, End}});
Error ->
Error
end;
parse_opts([{files, NewFiles} | Rest], Files, Exclude, Config) ->
parse_opts(Rest, expand_files(NewFiles, Files), Exclude, Config);
parse_opts([{exclude_files, NewExcludes} | Rest], Files, Exclude, Config) ->
Expand Down Expand Up @@ -361,20 +395,23 @@ resolve_config(
verbose = PreferVerbose,
print_width = PreferWidth,
pragma = PreferPragma,
out = PreferOut
out = PreferOut,
range = PreferRange
},
#config{
verbose = DefaultVerbose,
print_width = DefaultWidth,
pragma = DefaultPragma,
out = DefaultOut
out = DefaultOut,
range = DefaultRange
}
) ->
#config{
verbose = PreferVerbose orelse DefaultVerbose,
print_width = resolve_width(PreferWidth, DefaultWidth),
pragma = resolve_pragma(PreferPragma, DefaultPragma),
out = resolve_out(PreferOut, DefaultOut)
out = resolve_out(PreferOut, DefaultOut),
range = resolve_range(PreferRange, DefaultRange)
}.

resolve_width(undefined, W) -> W;
Expand All @@ -386,6 +423,9 @@ resolve_pragma(P, _) -> P.
resolve_out(standard_out, O) -> O;
resolve_out(O, _) -> O.

resolve_range(undefined, R) -> R;
resolve_range(R, _) -> R.

specified_files(List) ->
lists:filter(
fun
Expand Down
11 changes: 9 additions & 2 deletions test/erlfmt_cli_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
smoke_test_stdio_reinsert_pragma_config/1,
smoke_test_stdio_unicode/1,
smoke_test_stdio_check/1,
exclude_check/1
exclude_check/1,
range_check/1
]).

suite() ->
Expand Down Expand Up @@ -90,7 +91,8 @@ groups() ->
smoke_test_stdio_reinsert_pragma_config,
smoke_test_stdio_unicode,
smoke_test_stdio_check,
exclude_check
exclude_check,
range_check
]}
].

Expand Down Expand Up @@ -236,6 +238,11 @@ exclude_check(Config) when is_list(Config) ->
?assertNotMatch(nomatch, string:find(WithoutBroken, "[warn]")),
?assertMatch(nomatch, string:find(WithoutBroken, "broken.erl")).

range_check(Config) when is_list(Config) ->
slaykachu marked this conversation as resolved.
Show resolved Hide resolved
%% Simply check the options is properly recognized.
%% Here the range is the whole file.
stdio_test("attributes.erl", "--range=1,56", Config).

%%--------------------------------------------------------------------
%% HELPERS

Expand Down