Is ShowAppliedPatchAction really neccessary?

[Naveenaidu]

When the DIFF_EXCERPT_MAX_SIZE is greater than 4, then both Show Patch and Show Applied Patches is shown. The output shown by both is almost the same.

scibase-ieee-V3.py
[   1] ##·This·script·combines·both·extact_article_from·issue.py·and·Extract_Issue_Links.py
**** PEP8Bear [Section: cli | Severity: NORMAL] ****
!    ! The code does not comply to PEP8.
!    ! +1 -1 in /home/theprophet/test-coala/scibase-ieee-V3.py
[    ] *0. Do (N)othing
[    ]  1. (O)pen file
[    ]  2. (A)pply patch
[    ]  3. (S)how patch
[    ]  4. Add (I)gnore comment
[    ]  5. Show Applied (P)atches
[    ]  6. (G)enerate patches
[    ] Enter number (Ctrl-D to exit): 3

--> The sample output when (S)how Patch is selected

[----] /home/theprophet/test-coala/scibase-ieee-V3.py
[++++] /home/theprophet/test-coala/scibase-ieee-V3.py
[   1] ## This script combines both extact_article_from issue.py and Extract_Issue_Links.py
[   1] # This script combines both extact_article_from issue.py and Extract_Issue_Links.py
[    ] Displayed patch successfully.

--> The sample output when Show Applied (P)atches is selected

**** PEP8Bear [Section: cli] ****
**** Action Applied: ShowPatchAction ****

!    ! [Severity: NORMAL]
[----] /home/theprophet/test-coala/scibase-ieee-V3.py
[++++] /home/theprophet/test-coala/scibase-ieee-V3.py
[   1] ## This script combines both extact_article_from issue.py and Extract_Issue_Links.py
[   1] # This script combines both extact_article_from issue.py and Extract_Issue_Links.py

**************

If I am not wrong ShowAppliedPatchesAction() loops through all the applied actions and then actually calls ShowPatchAction() and then adds extra bit of information to the output of ShowPatchAction(). Is a seperate action for this work really necessary? Why not incorporate the action done by ShowAppliedPatchesAction() into the ShowPatchAction(). I think the Show Applied Patches is a redundant option.

Status:- Needs Discussion.

[Naveenaidu]

cc @Makman2 Need your opinion on this. And if this is a valid issue, i would like to work on it. 😄

[nityeshaga]

If I am not wrong ShowAppliedPatchesAction() loops through all the applied actions and then actually calls ShowPatchAction() and then adds extra bit of information to the output of ShowPatchAction()

Yes, you are right about this part. 😄

I think the Show Applied Patches is a redundant option.

This one, I beg to differ.

Actually ShowAppliedPatchesAction() was added pretty recently (last GSoC only). This action was added in a project that aimed to improve the coala CLI to make coala easier to use.

What ShowPatchAction() does is that it gives the user information about the patch that coala is currently suggesting for the current Result. Note: this is a patch that has not been applied yet which means that the Diff that it deals with has not been resolved yet. So, a user may decide to execute the ShowPatchAction() when he/she wants more information about the currently suggested patch.

On the other hand, ShowAppliedPatchesAction() is used to give more info about the patches that the user has previously chosen to apply upon the current Result.

So, in a way, ShowAppliedPatchesAction() is a view into the past whereas ShowPatchAction() is a glimpse into the future 😉

Therefore, IMHO I think that ShowAppliedPatchesAction() is very much required in order to make the user experience smoother.

Of course, I may be wrong about it but this is what I have understood. 😃

[nityeshaga]

Also the reason why you might be getting similar outputs for both these patches is because you probably executed ShowPatchAction first. This updated the applied_patches attribute of the concerned Result object and added ShowPatchAction to the list of applied_patches. Now, when you execute ShowAppliedPatchAction, coala shows you the patches from applied_patches list (which contains ShowPatchAction in it 😉)

[Naveenaidu]

@nityeshaga Thanks for the explanation 😄 . But if what you say about ShowAppliedPatches is correct could you solidify your explanation by any working example then it would be really nice and we could close this issue 😄

 On the other hand, ShowAppliedPatchesAction() is used to give more
 info about the patches that the user has previously chosen to apply
 upon the current Result.

Can you please explain the above point. ☝️ .

1. Do you mean to say that Show Applied Patches shows all the patches that been applied to the file
   until then? If yes, then Show Applied Patches doesn't is unable to achieve the required action. I
   tried applying multiple patches to the file and then selected Show Applied Patches, It still shows
   the same output as that of Show Patch Action.
2. Or does It means that it shows the patches that have been already applied to the Result object. If
   yes, then how is it actually possible. Doesn't coala go to the next set of lines once a particular patch
   has been applied to the current lines.

IMHO ShowAppliedPatches should show all the patches that have been applied until the correct instance.

Sorry! If i got this all wrong, I am new to coala and it still will take me time to get acquainted with the actual working mechanism of coala. 😅

[ishanSrt]

This issue need to fix ShowAppliedPatchesAction to show all the aggregated patches that have been applied till the called instant of that coala run. ShowPatchAction is working fine.

[ishanSrt]

Some user may want to check what are the patches he has applied until now, this must have been added keeping that in mind

[Naveenaidu]

@ishanSrt Can we conclude that ShowAppliedPatches is a necessary option but it doesn't work? And if that's the case @Makman2 I would like to be assigned to this issue 😄

[Naveenaidu]

It doesn't show all the previous patches applied until that point. All it shows is the patch for the current set of lines.

[Naveenaidu]

@Makman2 Is the issue still valid? Can I still work on it?

[Naveenaidu]

@jayvdb Since there are many overlapping PR's which are related to this issue. Can You please add an appropriate tag for this issue. And if the issue is not valid, can you please close the issue.