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

directed_changes #18

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

directed_changes #18

wants to merge 6 commits into from

Conversation

ThomasWitte
Copy link
Collaborator

Helper functions to choose an alternative between different possible SourceChanges. Used in development version of the interactive_script plugin.

@codecov
Copy link

codecov bot commented Sep 8, 2020

Codecov Report

Merging #18 into master will decrease coverage by 1.37%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
- Coverage   31.70%   30.32%   -1.38%     
==========================================
  Files          20       20              
  Lines        1782     1863      +81     
==========================================
  Hits          565      565              
- Misses       1217     1298      +81     
Impacted Files Coverage Δ
include/MiniLua/sourcechange.hpp 63.63% <ø> (ø)
src/core/sourcechange.cpp 14.16% <0.00%> (-28.34%) ⬇️
src/core/sourceexp.cpp 4.46% <0.00%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54e36e9...7b607d6. Read the comment docs.

src/core/sourcechange.cpp Outdated Show resolved Hide resolved
src/core/sourcechange.cpp Outdated Show resolved Hide resolved
@Lythenas
Copy link
Collaborator

Lythenas commented Sep 8, 2020

What exactly are the labels and hints and are they different?

@ThomasWitte
Copy link
Collaborator Author

What exactly are the labels and hints and are they different?

I added some comments to clarify the function a little bit. I'm still not content with the naming, but "hints" or "identifier" should be a name for a source location. The best and most intuitive naming I could find for a source location is the identifier of the first variable the value is bound to. If the value is never bound to a name this might be empty.
Labels should be a way to differentiate and enumerate source change variants, as a call to forceValue may yield multiple possible changes to different source locations. Currently this label simply consists of the location string and hint of the changed location.

@Lythenas
Copy link
Collaborator

Lythenas commented Sep 9, 2020

  1. I would create a new type for this. Something like SuggestedSourceChange that contains the hint/label/identifier string and the actual source change. E.g.
struct SuggestedSourceChange {
  std::string hint;
  std::shared_ptr<SourceChange> source_change;
}
  1. Then I would change source_change_labels to something like suggest_source_changes and return a list of the SuggestedSourceChange.
  2. The default source change would then be the first in the list (or can be selected by other logic e.g. in the gui). So the function default_source_change_label is also no longer needed.
  3. Then the get_sc_for_hint function is also no longer needed because the source change is part of the SuggestedSourceChange.

Do we really need to insert the $ if we already select the exact source change to apply?

Also something to think about: There might be groups of source changes that should be applied together. E.g.

x = 1
y = 2
z = x + y -- want to change z to 5

In this case we could do one of the following changes:

  • x = 3
  • y = 4
  • x = 2 and y = 3

But of course this could be divided by arbitrarily tiny differences (e.g. x = 1.1 and y = 3.9) so might no be feasible.

@ThomasWitte
Copy link
Collaborator Author

The problem is that source changes are currently a tree of SourceAssignments with SourceChangeAnd and SourceChangeOr as inner nodes. Changing this to a list of alternatives would probably be possible but definitely a major change and in the worst case lead to exponentially? (in the number of literals in the program) increased size in memory.

I agree that a proper type for the hint is needed (maybe the source has no textual representation etc) and a kind of iterator over the source change tree. It should support different heuristics to prioritize the alternatives (currently, it does a depth first search and chooses the first result).

I like the concept of inserting $ operators into the program as this way, the choice of alternatives has no internal state. All state is explicit in the program (which is also one premise of the direct manipulation that is directly reflected in the code). The problem is, that $ completely removes the possibility from the source change tree so the previous alternative is not available anymore. I plan to change that in the future (maybe) using an alternative semantics to $ that does not delete the source location information but instead just marks the alternative as impossible to apply to the program.

@ThomasWitte
Copy link
Collaborator Author

ThomasWitte commented Sep 9, 2020

In this case we could do one of the following changes:

  • x = 3
  • y = 4
  • x = 2 and y = 3

But of course this could be divided by arbitrarily tiny differences (e.g. x = 1.1 and y = 3.9) so might no be feasible.

It already does only the first and second alternative, roughly like this:

SourceChangeOr(SourceAssignment(1->3), SourceAssignment(2->4))

which defaults to the first alternative SourceAssignment(1->3). Inserting $ before 1 results then results in:

SourceChangeOr(SourceAssignment(2->4))

defaulting to the second alternative SourceAssignment(2->4)

@Lythenas
Copy link
Collaborator

Lythenas commented Sep 9, 2020

I like the concept of inserting $ operators into the program as this way, the choice of alternatives has no internal state. All state is explicit in the program (which is also one premise of the direct manipulation that is directly reflected in the code).

What do you mean by internal state? You have source code and a source change and get new source code back. I don't see any internal state there.

Or do you mean when you want to change the value multiple times (e.g. by dragging a line)? Then the GUI could simply save the location of the literal that should change and use that to select correct SourceChange the next time it wants to apply it. I'm not sure if this is currently possible.

@ThomasWitte
Copy link
Collaborator Author

I like the concept of inserting $ operators into the program as this way, the choice of alternatives has no internal state. All state is explicit in the program (which is also one premise of the direct manipulation that is directly reflected in the code).

What do you mean by internal state? You have source code and a source change and get new source code back. I don't see any internal state there.

I want to choose the literal that is changed before the marker is moved. One way I could think of is using the cursor position in the editor as a way to specify which location should be changed if possible (here the cursor position is the state). Another way that is a context menu to choose the alternative. In this case I can either internally save which entry is currently chosen or I make this state explicit by inserting $.

Or do you mean when you want to change the value multiple times (e.g. by dragging a line)? Then the GUI could simply save the location of the literal that should change and use that to select correct SourceChange the next time it wants to apply it. I'm not sure if this is currently possible.

Yes, I could save it in the GUI. But I don't like that I cannot save or see that. Another technical reason is that reexecuting the program invalidates most internal data structures so that I would have to match the chosen location to the new source locations somehow (I already have this problem using hints and the solutions are not very elegant). Perhaps tree-sitter solves this problem in the future.

@Lythenas
Copy link
Collaborator

Lythenas commented Sep 9, 2020

I feel like this is getting off-topic but for an easy to use GUI I would imagine something like:

  • the user clicks on the element he wants to change
  • editor highlights what code location would be changing and also what other locations could be selected
  • optional: user selects the location he wants or keeps the default. this can be done in different ways (e.g. dropdown, or clicking source location)
  • user drags the previously selected element (can stop and start any time)
  • only the selected source location updates until the user selects a different location or selects a different element

Here the GUI needs to

  • get a list of source locations that can change for highlighting (this could be done by acting like we would change something but not actually applying the changes).
  • remember the source location the user selects
  • choose the source change that corresponds to the selected source location when applying the change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants