-
Notifications
You must be signed in to change notification settings - Fork 235
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
Implementation to get list of possible jumps #1891
Open
PizieDust
wants to merge
5
commits into
ocaml:main
Choose a base branch
from
PizieDust:jump_enhanced
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
f8cef4a
get_all implementation to get list of possible jumps
PizieDust c91b01b
add changelog
PizieDust 21092f5
refactor
PizieDust 4a3c57f
don't raise exception when match-prev or match-next is not possible
PizieDust cd59943
update test
PizieDust File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,8 +95,6 @@ let rec find_map ~f = function | |
|
||
exception No_matching_target | ||
exception No_predicate of string | ||
exception No_next_match_case | ||
exception No_prev_match_case | ||
|
||
(* Returns first node on the list matching a predicate *) | ||
let rec find_node preds nodes = | ||
|
@@ -134,37 +132,42 @@ let find_case_pos cases pos direction = | |
in | ||
if check then Some pat_loc.loc_start else find_pos pos tail direction | ||
in | ||
let case = find_pos pos cases direction in | ||
match case with | ||
| Some location -> `Found location | ||
| None -> ( | ||
match direction with | ||
| Next -> raise No_next_match_case | ||
| Prev -> raise No_prev_match_case) | ||
find_pos pos cases direction | ||
|
||
let get typed_tree pos target = | ||
let get_enclosings typed_tree pos = | ||
let roots = Mbrowse.of_typedtree typed_tree in | ||
let enclosings = | ||
match Mbrowse.enclosing pos [ roots ] with | ||
| [] -> [] | ||
| l -> List.map ~f:snd l | ||
in | ||
let all_preds = | ||
[ ("fun", fun_pred); | ||
("let", let_pred); | ||
("module", module_pred); | ||
("module-type", module_type_pred); | ||
("match", match_pred); | ||
("match-next-case", match_pred); | ||
("match-prev-case", match_pred) | ||
] | ||
in | ||
|
||
match Mbrowse.enclosing pos [ roots ] with | ||
| [] -> [] | ||
| l -> List.map ~f:snd l | ||
|
||
let get_node_position target pos node = | ||
match target with | ||
| "match-next-case" -> find_case_pos (get_cases_from_match node) pos Next | ||
| "match-prev-case" -> | ||
find_case_pos (List.rev (get_cases_from_match node)) pos Prev | ||
| _ -> | ||
let node_loc = Browse_raw.node_real_loc Location.none node in | ||
Some node_loc.Location.loc_start | ||
|
||
let predicates = | ||
[ ("fun", fun_pred); | ||
("let", let_pred); | ||
("module", module_pred); | ||
("module-type", module_type_pred); | ||
("match", match_pred); | ||
("match-next-case", match_pred); | ||
("match-prev-case", match_pred) | ||
] | ||
|
||
let get typed_tree pos target = | ||
let enclosings = get_enclosings typed_tree pos in | ||
let targets = Str.split (Str.regexp "[, ]") target in | ||
try | ||
let preds = | ||
List.map targets ~f:(fun target -> | ||
match | ||
List.find_some all_preds ~f:(fun (name, _) -> name = target) | ||
List.find_some predicates ~f:(fun (name, _) -> name = target) | ||
with | ||
| Some (_, f) -> f | ||
| None -> raise (No_predicate target)) | ||
|
@@ -173,18 +176,28 @@ let get typed_tree pos target = | |
else | ||
let nodes = skip_non_moving pos enclosings in | ||
let node = find_node preds nodes in | ||
match target with | ||
| "match-next-case" -> find_case_pos (get_cases_from_match node) pos Next | ||
| "match-prev-case" -> | ||
find_case_pos (List.rev (get_cases_from_match node)) pos Prev | ||
| _ -> | ||
let node_loc = Browse_raw.node_real_loc Location.none node in | ||
`Found node_loc.Location.loc_start | ||
match get_node_position target pos node with | ||
| Some loc -> `Found loc | ||
| None -> `Error ("No matching case found for " ^ target) | ||
with | ||
| No_predicate target -> `Error ("No predicate for " ^ target) | ||
| No_matching_target -> `Error "No matching target" | ||
| No_next_match_case -> `Error "No next case found" | ||
| No_prev_match_case -> `Error "No previous case found" | ||
|
||
let get_all typed_tree pos = | ||
let enclosings = get_enclosings typed_tree pos in | ||
let nodes = skip_non_moving pos enclosings in | ||
let results = | ||
List.filter_map | ||
~f:(fun (target, pred) -> | ||
match find_node [ pred ] nodes with | ||
| exception No_matching_target -> None | ||
| node -> ( | ||
match get_node_position target pos node with | ||
| Some position -> Some (target, position) | ||
| None -> None)) | ||
Comment on lines
+195
to
+197
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wdyt about | node ->
Option.map
(fun pos -> (target, pos))
get_node_position target pos node) |
||
predicates | ||
in | ||
results | ||
|
||
let phrase typed_tree pos target = | ||
let roots = Mbrowse.of_typedtree typed_tree in | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that it is not particularly related to this PR but just out of curiosity, why going for a polymorphic variant? It looks like
result
is sufficient here no?