Gasman2014 / KiCad-Diff

Scripts for performing image diffs between pcbnew layout revisions
MIT License
236 stars 41 forks source link

Improve interface, plot display, and textual diff #36

Open Gasman2014 opened 3 years ago

Gasman2014 commented 3 years ago

The current paradigm relies on selection of commits ante hoc and the script has to be rerun from scratch to compare different revisions.

A more useful real world scenario would be to review the sequential changes between arbitrary commits, the whole interface being delivered within a web-browser. It would probably be easiest to use a git (or fossil) commit-hook to generate the artefacts after every commit so the artefacts were already embedded in the repo. This does slightly go against the philosophy that the board is 'source code' and the images are 'compiled' so should not pollute the repo but would simplify & speed up the plotting process.

Options to select multiple layers would be helpful. The DOM would have to be redesigned to support javascript selection of commits/layers and to explore the actual diff changes.

The initial launch script could be an 'Action script' from within pcbnew.

Ultimately, it would be very useful to support schematic diffs (programatic plotting of schematics may be available in KiCad 6) and to be able to launch this from within the Project Manager window.

A very rough initial mockup --->

Screenshot 2021-02-17 at 13 40 46

leoheck commented 3 years ago

Ultimately, it would be very useful to support schematic diffs (programmatic plotting of schematics may be available in KiCad 6) and to be able to launch this from within the Project Manager window.

Oh, I was talking about this in another issue. Nice.

This idea is pretty nice. I would like to see this happening.

It would probably be easiest to use a git (or fossil) commit-hook to generate the artefacts after every commit so the artefacts were already embedded in the repo.

I don't think it is a good idea to store these temporary things.

If could decide, I would let the tool generate every commit on the fly, when the user selects it. So, the user starts by selecting 2 arbitrary commits, and then the web interface starts showing the diffs of these 2 commits.

But the user can change pick a new commit on the fly, by changing the 1st commit or the 2nd commit which will make the tool to run again generating a new diff.

Also, we could also have a checkbox to calculate all diffs from all commits at one in a single run, too.

What do you need to start developing this? How can I we help?

leoheck commented 3 years ago

I made a script to test this. Running in a small board/repo it does not take too much time. It is taking about 5s to generate 6 comparisons.

Check here how it looks, but not good as your mockup, however, it is functional.
image

I am not sure the script is totally compatible with mac, but it is here if you would like to give it a try. Just put it inside Kicad-diff folder, then execute source env.sh And you can use it with kidiff-multi-git ./board.kicad_pcb https://www.dropbox.com/s/7w4wi5tskxy6i7u/kidiff-multi-git?dl=0

Gasman2014 commented 3 years ago

Not totally working on macOS but I see the principle - however, it would only seem to allow sequential comparisons? v4 to v5 for instance but not v2 to v4? I think a means of selecting arbitrary comparisons might be more useful? I fully understand your objection to string the artefacts in the repo on each commit - the only reason I was considering going down this route was because (1) it would be easy to to do this using a commit hook and (2) it would be (relatively) easy to pull up the pre-made files using javascript in the web browser (3) I know next to no javascript :( . It would also be fast. If we don't have pre-made artefacts, I think we need to write a backend web server with an API to generate the artefacts on the fly. I agree this would be technically better and a route I would prefer as it is probably more supportable long term and it would also avoid having to rewrite the textural diff comparison in javascript.

Not sure about generating all diffs from all commits - wouldn't that mean an O(n^2) function? For a big repo that might be very intensive.

I have a lot on my plate at the moment, unfortunately (family bereavement & some other medical issues) so I'm not in a good place to dig in at present.

leoheck commented 3 years ago

Yes, totally.

This was just a fast test to achieve something close to as you were describing by reusing kidiff as it is.

Another way to reuse it, enabling us to evaluate this idea faster is to generate a page with 2 columns of commits and then, calling kidiff on the fly by clicking a button.

I will check if I can do something similar by calling an external script from inside the main page.

leoheck commented 3 years ago

Second test... now generating the whole set of comparisons... 49 comparisons took 52 seconds here. The Script is here if anyone wants to check. There is one shortcut for the Compare button it is the character c

image

leoheck commented 3 years ago

What is the FP text on your initial mockup?

Gasman2014 commented 3 years ago

That looks very good and fairly reasonable in terms of time. I still think an on the fly comparison is still going to be necessary as repo size increases to keep it responsive. For the testing phase, you could shave some time off by checking if the output artefacts exist already before regenerating but ultimately, moving to a server will be the answer.

Is it possible to get more of the context into the commit column? i.e could you also include date and commit title? Have you looked at trying to display the commit list as a single column with checkboxes. I think some javascript like this can ensure that you only have two selected.

$('input[type=checkbox]').on('change', function (e) { if ($('input[type=checkbox]:checked').length > 2) { $(this).prop('checked', false); alert("allowed only 2”); } });

On 8 Mar 2021, at 00:57, Leandro Heck notifications@github.com wrote:

Second test... now generating the whole set of comparisons... 49 comparisons took 52 seconds here. The Script is here https://www.dropbox.com/s/dea68v4287ijecc/kidiff-multi-git2?dl=0 if anyone wants to check. There is one shortcut for the Compare button it is the character c

https://user-images.githubusercontent.com/1277920/110261614-b3e34900-7f8f-11eb-914b-cbcff5e43ab0.png — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Gasman2014/KiCad-Diff/issues/36#issuecomment-792394076, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADFOJZVKOMOEWXJRTOUWLTCQOGLANCNFSM4XYNIEMA.

Gasman2014 commented 3 years ago

FP Text was just part of an example text diff placeholder. I would like to see something that indicates by colour added/modified/removed with reference to modules and tracks etc. I did start to have a go at generating this directly via Python and not trying to parse the pcb file. For instance, It is very easy to get a list of board modules and positions directly.

John Pateman

Sent from my iPhone

On 8 Mar 2021, at 10:33, John Pateman johnpateman@me.com wrote:

That looks very good and fairly reasonable in terms of time. I still think an on the fly comparison is still going to be necessary as repo size increases to keep it responsive. For the testing phase, you could shave some time off by checking if the output artefacts exist already before regenerating but ultimately, moving to a server will be the answer.

Is it possible to get more of the context into the commit column? i.e could you also include date and commit title? Have you looked at trying to display the commit list as a single column with checkboxes. I think some javascript like this can ensure that you only have two selected.

$('input[type=checkbox]').on('change', function (e) { if ($('input[type=checkbox]:checked').length > 2) { $(this).prop('checked', false); alert("allowed only 2”); } });

On 8 Mar 2021, at 00:57, Leandro Heck notifications@github.com wrote:

Second test... now generating the whole set of comparisons... 49 comparisons took 52 seconds here. The Script is here if anyone wants to check. There is one shortcut for the Compare button it is the character c

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or unsubscribe.

Gasman2014 commented 3 years ago

Something like this on the RHS is what I had in mind.

On 8 Mar 2021, at 11:53, John Pateman johnpateman@me.com wrote:

FP Text was just part of an example text diff placeholder. I would like to see something that indicates by colour added/modified/removed with reference to modules and tracks etc. I did start to have a go at generating this directly via Python and not trying to parse the pcb file. For instance, It is very easy to get a list of board modules and positions directly.

John Pateman

Sent from my iPhone

On 8 Mar 2021, at 10:33, John Pateman johnpateman@me.com wrote:

That looks very good and fairly reasonable in terms of time. I still think an on the fly comparison is still going to be necessary as repo size increases to keep it responsive. For the testing phase, you could shave some time off by checking if the output artefacts exist already before regenerating but ultimately, moving to a server will be the answer.

Is it possible to get more of the context into the commit column? i.e could you also include date and commit title? Have you looked at trying to display the commit list as a single column with checkboxes. I think some javascript like this can ensure that you only have two selected.

$('input[type=checkbox]').on('change', function (e) { if ($('input[type=checkbox]:checked').length > 2) { $(this).prop('checked', false); alert("allowed only 2”); } });

On 8 Mar 2021, at 00:57, Leandro Heck <notifications@github.com mailto:notifications@github.com> wrote:

Second test... now generating the whole set of comparisons... 49 comparisons took 52 seconds here. The Script is here https://www.dropbox.com/s/dea68v4287ijecc/kidiff-multi-git2?dl=0 if anyone wants to check. There is one shortcut for the Compare button it is the character c

https://user-images.githubusercontent.com/1277920/110261614-b3e34900-7f8f-11eb-914b-cbcff5e43ab0.png — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Gasman2014/KiCad-Diff/issues/36#issuecomment-792394076, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADFOJZVKOMOEWXJRTOUWLTCQOGLANCNFSM4XYNIEMA.

leoheck commented 3 years ago

Since the last test, I was trying to make your mockup alive by making the initial Html This the thing I was able to made yesterday night. I was thinking that having the layers view under the commits will be better to give more space to the image and textual diff under the board. Or, the 3 comparison view in the center, and then a column with the textual diff image

For the on-the-fly comparison, I still want to learn how, and if it is possible to run the script from the javascript, then nothing much will be needed to change for this test.

This is also what I started to look at today, thanks I will try to embed this behavior too.

$('input[type=checkbox]').on('change', function (e) {
    if ($('input[type=checkbox]:checked').length > 2) {
        $(this).prop('checked', false);
        alert("allowed only 2”);
    }
});
leoheck commented 3 years ago

Getting better. I was using radio instead of checkboxes. But the javascript to limit in 2 is not working yet.

image

leoheck commented 3 years ago

Experimenting while waiting for other stuff to be finished. image

leoheck commented 3 years ago

image

leoheck commented 3 years ago

Now it is constraining the commits selection in 2 Here is the page https://www.dropbox.com/s/tre0bvvu1gesmcy/main2.html?dl=0

Gasman2014 commented 3 years ago

That looks great - I like that very much. I can't download anything from the link though.

leoheck commented 3 years ago

The link should be working now. Shortcuts (K and L) to change the selected layer are working too. wget https://www.dropbox.com/s/tre0bvvu1gesmcy/main2.html -O main2.html

Gasman2014 commented 3 years ago

I wonder if we should still plan to move the layer selector underneath the main pane rather than in the left column (as in my original mockup)? Two reasons - (1) The number of diffs may be large and it would make it more likely that both were in view and (2) Ultimately I want to use this to view schematic diffs too so keeping the left side consistent would be important. If we keep this layer selector on the left column, what would be the equivalent for a schematic diff?

I can select the layers using the K + L keys but I am unable to select the commits with the arrows. I can select two (and only two) with the mouse, which is fine. (Safari on macOS).

My thoughts are that the right column should be a more simplified version of the text diff.

leoheck commented 3 years ago

Shortcuts on the commits list are not working yet. I have to think about what to do since we have 2 selections. For instance which one will change? How to move the other one? But these things are not important right now since we are just discussing how this is going to be.


For the layers list, I don't like them on the right side the way your mockup is. Maybe a big and list on the right as it appears in Kicad would be something to think about.


Considering the commits list, in my repos I don't have a huge list of commits to make the commits list grow the whole height of the pages. This is why I was thinking to have something under this commits list.

Also, if the user selects an initial commit and the final commit as the boundaries for this tool... let's say the whole list of commits on a PR, this list is going to be short, most of the time, I think. So, in the mean case, we are going to waste that space.


If the main goal is to have the schematics together, maybe it is good to add this to the mockups too. Could you draw something to exemplify how it could be?

Gasman2014 commented 3 years ago

OK, thanks - noted. I agree that the navigation is possibly tricky but not the most pressing. I am sure that we can come up with some suitable mechanism.

The right hand column for PCB diffs was going to be a pared down version of the Text Diff that is in the tabbed boxes at the bottom of the current diff. i.e. this bit from the current diff strategy but refactored a bit.

I agree that we don’t want to waste space and yes, most of the time there are not too many diffs and, moreover, you would generally be interested in a small, close range of diffs - i.e what changed between the three commits between yesterday and today. However, I do have several boards that have >25 commits and I am sure that some complex designs may well have significantly more than that; arguably, the more complex the history, the more useful this sort of tool is. Anyhow, it is not critical as the exact arrangements of blocks is probably fairly easy to sort.

I was intending to keep the left column as a fixed point for both schematic diff and pcb diff and change the right two panes.

I was having a look at allspice.io (for Altium) for some inspiration.

This is a very basic mockup of the sort of layout I had in mind.

On 10 Mar 2021, at 13:12, Leandro Heck @.***> wrote:

Shortcuts on the commits list are not working yet. I have to think about what to do since we have 2 selections. For instance which one will change? How to move the other one? But these things are not important right now since we are just discussing how this is going to be.

For the layers list, I don't like them on the right side the way your mockup is. Maybe a big and list on the right as it appears in Kicad would be something to think about.

Considering the commits list, in my repos I don't have a huge list of commits to make the commits list grow the whole height of the pages. This is why I was thinking to have something under this commits list.

Also, if the user selects an initial commit and the final commit as the boundaries for this tool... let's say the whole list of commits on a PR, this list is going to be short, most of the time, I think. So, in the mean case, we are going to waste that space.

If the main goal is to have the schematics together, maybe it is good to add this to the mockups too. Could you draw something to exemplify how it could be?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Gasman2014/KiCad-Diff/issues/36#issuecomment-795392047, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADFOIYPON2LV7KUT2I3STTC5V2XANCNFSM4XYNIEMA.

leoheck commented 3 years ago

Oh, this allspice.io is good. Let me rethink this then.

leoheck commented 3 years ago

Improved the commits column. https://www.dropbox.com/s/vei4mtbxemdbr42/main3.html?dl=0

The list is not following the focus yet.

leoheck commented 3 years ago

Not beautiful yet, but it will be really good to review boards. image

Being able to pass through each commit with the keyboard is soo good.

One thing that I have noticed is that we need to mark commits that have changed in the board. The same will happen for the schematic.

For instance, I have a huge list of commits on this project. But just a few for the boards... and then we won't be able to see differences in many of these items...

I am thinking that it will be needed to have near each commit an icon for these things...

leoheck commented 3 years ago

@Gasman2014 check this video https://www.dropbox.com/s/pt2ppumr2qxtoss/Screencast%202021-03-14%2000%3A53%3A15.mp4?dl=0

Here is a screen recording of the current status. I start by moving commits, both together. Then changing layers. All with the arrow keys Then after changing only the second commit. Pretty nice.

I think the layers list has to be inside the diff area, but I just put it where it is to have something to test quickly.

Gasman2014 commented 3 years ago

Hey, this is beginning to look pretty cool and more functional. I'm impressed!

For instance, I have a huge list of commits on this project. But just a few for the boards... and then we won't be able to see differences in many of these items...

Yes, this could be a problem. At present, we identify differences between two selected commits and bail out if the two files are identical. How are you building your commit list? I presume that you iterate through each commit to generate a set of svgs for each commit. If you iterate in chronological order (oldest first) and compare the current .kicad_pcb (and in future development, .kicad_sch) with the previous one, you should identify where a change took place. I would favour flagging these somehow in the list of commits. The simplest solution is to flag with [S] (schematic change) or [P] (pcb change? or maybe [L] layout change). It would make most sense to flag the later commit I guess (arrow of time). KiCad has got better at not moving random chunks of text around so diff equivalence is easier than it used to be.

I think the layers list has to be inside the diff area, but I just put it where it is to have something to test quickly.

I agree. Do you think we should try to retain the three pane layout or not i.e the composite with the two originals beneath? I wonder if we had a selector for V1, V2 and the composite that that might be better? GitLab does a graphic diff offering 2-up/swipe/onionskin. I think the composite colorMatrix works well but the whole layout then takes up a lot of vertical space. Certainly moving the Texttural diff off tot the right column will help a bit but offering an alternative view might be better - especially if there is a need to accommodate the layer info in this section too.

Screenshot 2021-03-14 at 13 00 18

leoheck commented 3 years ago

We should do something to flags these changes. This was just something that I have noticed to keep in mind. For instance, examples above and the video were made by listing commits of the layout only, since we don't have any possible way to do these image diffs for the schematics yet. And also because I wanted a fast way to show something in this video.

Your simple solution for that is something I was thinking too.. maybe reusing the Schematic/Layout icons we have on top of the commits list. I was thinking to have these 2 buttons to change the schematic/layout views.

It would make most sense to flag the later commit I guess (arrow of time). KiCad has got better at not moving random chunks of text around so diff equivalence is easier than it used to be.

I didn't get this part. But I think it is just a matter of getting with the SCM the list of the files that were changed on each commit filtering schematics and layout only and marking these in the commits list.

Do you think we should try to retain the three-pane layout or not i.e the composite with the two originals beneath?

I am not sure, I have added it there since it was in the previous Kidiff. But they don't add any information, since they are small. We can also iterate back and forth in the commits list, I agree these 2 views are not needed anymore.

I wonder if we had a selector for V1, V2, and the composite that might be better?

Yeah, true, this is a good idea. This Gitlab Image diff feature could be an interesting feature here. For both layout and schematic views.

leoheck commented 3 years ago

Checking how to generate schematic diffs easily with https://github.com/jnavila/plotkicadsch for now, while Kicad can't do that. This works nice. Of course, it will need tweaks on the generated svg's but it makes me dream. haha

I am going to create a repo with this mockup, then you and others could test this easily

image

Gasman2014 commented 3 years ago

It would make most sense to flag the later commit I guess (arrow of time). KiCad has got better at not moving random chunks of text around so diff equivalence is easier than it used to be. I didn't get this part. But I think it is just a matter of getting with the SCM the list of the files that were changed on each commit filtering schematics and layout only and marking these in the commits list

I was only meaning that you tag the chronologically later commit. Commit 1 (made first) is different to Commit 2 (made later) so you tag Commit 2.

Historically, if you edited a component in KiCad, the whole block of text describing the module got removed and the edited version was appended to the end of the modules section. IIRC even if you edited but made no change this happened - I think it has been fixed in versions in the last couple of years but it might mean we should deprecate early versions of Kicad 5?

The schematic diff from @jnavila looks really good - I did get it running in the past but I am very unfamiliar with OCaml so there was a bit of a learning curve. Of course, the layers selection tool don't make much sense in this view.

Yeah, true, this is a good idea. This Gitlab Image diff feature could be an interesting feature here. For both layout and schematic views.

I think if we could get a git-style choice of comparison options then we could drop the bottom two images.

Gasman2014 commented 3 years ago

I had something like this in mind for schematics. Perhaps the switch between PCB and schematic above the Commits section.

SCH_DIFF_MOCK

leoheck commented 3 years ago

Experimenting to mark each commit when layout or schematic has changed. image

leoheck commented 3 years ago

This usable mockup is here https://github.com/leoheck/kdiff

leoheck commented 3 years ago

Let me know if you have any issue running it on the OSX

Gasman2014 commented 3 years ago

Had a quick look at this. Starting to look promising. Couple of observations.

The ’tac’ program is not available for macOS - I can’t even find the source to compile it. The same functionality is provided by ’tail -r’. Does that work for you?

xdg-open "${output_folder}/web/main.html" & 2> /dev/null || open "${output_folder}/web/main.html” also doesn't work on macOS - you just need the second option ‘open “${output_folder}/web/main.html’ on macOS.

Probably best to make this something like this (not sure about Windows here tbh..)

if [[ "$OSTYPE" == "linux-gnu" ]]; then xdg-open "${output_folder}/web/main.html" & 2> /dev/null elif [[ "$OSTYPE" == "darwin" ]]; then open "${output_folder}/web/main.html fi

Also need to use Cmd + Arrows for walk through on macOS (not Ctrl).

Now I get a web page with the layout but no diffs. :( I am getting this error in the terminal.

Generating 324 comparisons

  1. 169f729 - de64204
    • 789491b
    • 022b97a
    • 63ed15f
    • 413a8a3
    • 8a947fd
    • 9dfb565
    • 726b91e
    • 3830ce9
        • cb2673c
        • 2027849
        • aefc78e
        • c874af7
        • ec7c5a1
        • 157e86b
        • f8043af
        • 6370707 fatal: ambiguous argument '6370707 ': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- [...]' fatal: ambiguous argument '6370707 ': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- [...]' fatal: ambiguous argument '6370707 ': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- [...]' fatal: ambiguous argument 'f8043af ': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- [...]' fatal: ambiguous argument 'f8043af ': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- […]'
MORE OF THE SAME ERROR….

sed: 1: "kidiff/web/main.html": invalid command code k sed: 1: "kidiff/web/main.html": invalid command code k sed: 1: "kidiff/web/main.html": invalid command code k sed: 1: "kidiff/web/main.html": invalid command code k sed: 1: "kidiff/web/main.html": invalid command code k

Are you happy if we carry this on by email or do you want me to log these as issues on GitHub? (FWIW, my employer has a firewall block on GitHub at work (I work in the UK in the health service), so response might be quicker by email.

BW

John

On 15 Mar 2021, at 15:22, Leandro Heck @.***> wrote:

Let me know if you have any issue running it on the OSX

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Gasman2014 commented 3 years ago

Also, all the commit files are generated correctly in the kidiff directory, but the ’sch’ directory remains empty. Will have to have a further play with plotgitsch as I think it needs some further tinkering - it isn’t the easiest to get working and I seem to remember struggling in the past.

BW

On 15 Mar 2021, at 15:22, Leandro Heck @.***> wrote:

Let me know if you have any issue running it on the OSX

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Gasman2014/KiCad-Diff/issues/36#issuecomment-799506793, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADFOIHU5YLWR5EH27IHXDTDYQ4RANCNFSM4XYNIEMA.

leoheck commented 3 years ago

Commenting by email is fine.

Ctrl and Cmd are the same things for me... are talking about the labels, right? This Ctrl/Cmd thing is not working yet. Only the label is there. You can try it with [ ], I am using these characters instead of Ctrl-> Ctrl<- for tests. However, it is not working pretty fine yet.

I have fixed the tac and xdg-open, let me know if my changes fix this for you.

Ah, could you share the git --version here? Maybe you are using a different version, or maybe you are testing a version of the mockup that does not work fine, I had issues too hours ago.

plotkicadsch is so hard to install, I dont know why but..

gh repo clone jnavila/plotkicadsch
cd plotkicadsch
#
rm -rf  ~/.opam #  <======= warning, this is going to remove the orignal opam folder
#
opam switch create 4.09.1
opam switch 4.09.1
eval $(opam config env)
opam pin add kicadsch .
opam pin add plotkicadsch .
opam update
opam install plotkicadsch

Check if it works, it has to create this diff_board.svg only if schematics has diffs

eval $(opam config env)
cd [kicad_repo]
plotgitsch -l *-cache.lib -i "echo" HEAD HEAD~1 -k
Gasman2014 commented 3 years ago

Yeah, I have plotgitsch and plotkicadsch installed and running OK from the command line.

git --version git version 2.29.2

Making a bit more progress

Generating 36 comparisons

  1. c0a3b8e - 8b0696a
    • fc6818f
    • 064acea
    • cdb8005
    • ffb468c
  2. 8b0696a - c0a3b8e
    • fc6818f
    • 064acea
    • cdb8005
        • ffb468c
      1. fc6818f - c0a3b8e
        • 8b0696a
        • 064acea
        • cdb8005
        • ffb468c
      2. 064acea - c0a3b8e
        • 8b0696a
        • fc6818f
        • cdb8005
        • ffb468c
      3. cdb8005 - c0a3b8e
        • 8b0696a
        • fc6818f
        • 064acea
        • ffb468c
      4. ffb468c - c0a3b8e
        • 8b0696a
        • fc6818f
        • 064acea
        • cdb8005 fatal: ambiguous argument 'ffb468c ': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- [...]'

Is there an extraneous space in there?

John

On 15 Mar 2021, at 21:16, Leandro Heck @.***> wrote:

Commenting by email is fine.

Ctrl and Cmd are the same things for me... are talking about the labels, right? This Ctrl/Cmd thing is not working yet. Only the label is there. You can try it with [ ], I am using these characters instead of Ctrl-> Ctrl<- for tests. However, it is not working pretty fine yet.

I have fixed the tac and xdg-open, let me know if my changes fix this for you.

Ah, could you share the git --version here? Maybe you are using a different version, or maybe you are testing a version of the mockup that does not work fine, I had issues too hours ago.

plotkicadsch is so hard to install, I dont know why but..

check if you have opam 2.0 then try this gh repo clone jnavila/plotkicadsch cd plotkicadsch # rm -rf ~/.opam # <======= warning, this is going to remove the orignal opam folder # opam switch create 4.09.1 opam switch 4.09.1 eval $(opam config env) opam pin add kicadsch . opam pin add plotkicadsch . opam update opam install plotkicadsch Check if it works, it has to create this diff_board.svg only if schematics has diffs

eval $(opam config env) cd [kicad_repo] plotgitsch -l *-cache.lib -i "echo" HEAD HEAD~1 -k — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Gasman2014/KiCad-Diff/issues/36#issuecomment-799757978, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADFONNMPSWLVKKN3JYIZ3TDZ2MFANCNFSM4XYNIEMA.

leoheck commented 3 years ago

I will work on this issues regarding mac compatibility as soon as possible.

Also, I was able to tweak plotgitsch to generate individual svgs for each schematic, so kdiff can plot diff using trypich view. This Opam language is pretty hard haha.

I am having issues with the html+javascrip, when I solve this I will let you know so you can test it again.
Right now, this is the current status..

image

leoheck commented 3 years ago

Ah, I have added an extra icon to indicate diff in text files that are not sch and kicad_pcb

leoheck commented 3 years ago

Hey @Gasman2014 could you try the latest code on https://github.com/leoheck/kdiff ? There are some instructions on README.md if needed

To have schematics diff working, you will have to install my version of plotgitsch. This version generates svg for each commit being compared even if they don't have any difference. It won't generate dffs too.

Gasman2014 commented 3 years ago

Thanks for digging in with this. I have installed the patched version of plotgitsch from your GitHub repo and been trying to get this to run.

On each loop, the sch1.svg & sch2.svg are created, but in the root directory of the repo.

On some loops, a "sch-diff.svg" file is created, but this sch-diff.svg file is not actually an svg! It is actually a .png file!

dirname: illegal option -- f
usage: dirname path
basename: illegal option -- f
usage: basename string [suffix]
       basename [-a] [-s suffix] string [...]

Generating 72 comparisons

   1. 169f729 - de64204
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
   2.         - 789491b
   3.         - 022b97a
   4.         - aefc78e
usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file
       cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory
usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file
       cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory
cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.HtHKy6Jt: No such file or directory
rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.HtHKy6Jt: No such file or directory
cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.oJh7PFNO: No such file or directory
rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.oJh7PFNO: No such file or directory
cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.BXYPyvcT: No such file or directory
rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.BXYPyvcT: No such file or directory
cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.LS9saElt: No such file or directory
rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.LS9saElt: No such file or directory
cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.ZfS3D5od: No such file or directory
rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.ZfS3D5od: No such file or directory
cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.UyD3djba: No such file or directory
rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.UyD3djba: No such file or directory
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch1.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
sed: 1: "sch2.svg": unterminated substitute pattern
   5.         - c874af7

After this I get number of errors related to paths.

cp: /Users/main.html: No such file or directory
cp: /Users/kdiff.css: No such file or directory
cp: /Users/kdiff.js: No such file or directory
cp: /Users/blank.svg: No such file or directory

And finally

sed: 1: "kidiff/web/main.html": invalid command code k
sed: 1: "kidiff/web/main.html": invalid command code k
sed: 1: "kidiff/web/main.html": invalid command code k
sed: 1: "kidiff/web/main.html": invalid command code k

On 23 Mar 2021, at 20:01, Leandro Heck @.***> wrote:

Hey @Gasman2014 https://github.com/Gasman2014 could you try the latest code on https://github.com/leoheck/kdiff https://github.com/leoheck/kdiff ? There are some instructions on README.md if needed

To have schematics diff working, you will have to install my version of plotgitsch. This version generates svg for each commit being compared even if they don't have any difference. It won't generate dffs too.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Gasman2014/KiCad-Diff/issues/36#issuecomment-805198056, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADFOL3AV2H4WQAH4K55XLTFDXQ5ANCNFSM4XYNIEMA.

leoheck commented 3 years ago

Let me check this, thanks for the logs

leoheck commented 3 years ago

readlink_ was wrong. I think it is good for another try

leoheck commented 3 years ago

Some screenshots Zoom on Layout is not working yet, only on Schematic

Schematic image

Layout image

leoheck commented 3 years ago

Good, svg control is fixed.

Gasman2014 commented 3 years ago

OK, that looks much better and I am now able to trigger the script and the web page is opening correctly! Safari complains but schematic diffing does actually work on Firefox! Well done - great work!!

Initially I was seeing a lot of ’sed’ errors. On macOS, sed is provided by the OS and it is pretty ancient - annoyingly 'sed —version' doesn’t return a value but 'strings $(whence sed)' returns strings which say it dates from 9/8/2004! If I replace the ’sed’ with ‘gsed’ these errors disappear. I’m not sure that’s worth a pull request!

There is still a problem with the graphic pcb diff though. Looking at the console, I don't think you are passing the board name correctly. My board is ’ThermocoupleDatalogger-F_Cu.svg’ etc but the link is failing as it is looking for ‘board-F_Cu.svg’.When you refresh, the index page has the correct board name. I think you are passing “board-“ as fixed string in the js at around lines 265 and 300.

Because of this, I only get one graphical diff working (The F-Cu layer) and I can’t change layer.

The other issue is that there is a discrepancy between the commit selected and the actual commit displayed. In the second screenshot (see debugging console), I have [6370707] and [169F729] selected BUT [6370707] and [F8043AF] (i.e. 1st and second commits) are the ones that are shown.

Firefox

On 24 Mar 2021, at 00:59, Leandro Heck @.***> wrote:

Good, svg control is fixed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Gasman2014/KiCad-Diff/issues/36#issuecomment-805391380, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADFOLNASIILQNN7R35TS3TFE2QRANCNFSM4XYNIEMA.

Generating 56 comparisons

  1. 169f729 - de64204 mv: rename .svg to kidiff/169f729/.svg: No such file or directory
    • 789491b mv: rename .svg to kidiff/169f729/.svg: No such file or directory
    • 022b97a mv: rename .svg to kidiff/169f729/.svg: No such file or directory
    • aefc78e usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.7ajclJUn: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.7ajclJUn: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.7sNf1UzT: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.7sNf1UzT: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.QVUp2b1R: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.QVUp2b1R: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.OFtTLKGa: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.OFtTLKGa: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.HmHL4psq: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.HmHL4psq: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.BIaAdjCa: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.BIaAdjCa: No such file or directory mv: rename .svg to kidiff/169f729/.svg: No such file or directory
    • 27381ec usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.XeeLIcYO: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.XeeLIcYO: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.a4sa1js6: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.6qU4IGaf: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.a4sa1js6: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.6qU4IGaf: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.XlaBJmZt: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.XlaBJmZt: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.BlgrX0vN: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.BlgrX0vN: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.aXuUnRpZ: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.aXuUnRpZ: No such file or directory mv: rename .svg to kidiff/169f729/.svg: No such file or directory
    • 71dbe3b usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.4nwcimZY: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.4nwcimZY: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.xmcW37Uf: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.xmcW37Uf: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.q9tPHiVW: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.q9tPHiVW: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.aSE9Io4f: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.aSE9Io4f: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.HAtQmWic: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.HAtQmWic: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.h4Iln6Pu: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.h4Iln6Pu: No such file or directory mv: rename .svg to kidiff/169f729/.svg: No such file or directory
    • f338a5c usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.D8yeGjGj: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.D8yeGjGj: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.pt8X2yCn: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.pt8X2yCn: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.HlK9MG0Y: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.vW67U1eg: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.HlK9MG0Y: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.vW67U1eg: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.sGjIk9Xn: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.sGjIk9Xn: No such file or directory cp: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.trnB1vJC: No such file or directory rm: /var/folders/v9/4j9n8n1118n1605x0s_wf74m0000gn/T/tmp.trnB1vJC: No such file or directory mv: rename .svg to kidiff/169f729/.svg: No such file or directory
  2. de64204 - 169f729, skipping...
    • 789491b, skipping...
        • 022b97a, skipping...
        • aefc78e, skipping...
        • 27381ec, skipping...
        • 71dbe3b, skipping...
        • f338a5c, skipping...
      1. 789491b - 169f729, skipping...
        • de64204, skipping...
        • 022b97a, skipping...
        • aefc78e, skipping...
        • 27381ec, skipping...
        • 71dbe3b, skipping...
        • f338a5c, skipping...
      2. 022b97a - 169f729, skipping...
        • de64204, skipping...
        • 789491b, skipping...
        • aefc78e, skipping...
        • 27381ec, skipping...
        • 71dbe3b, skipping...
        • f338a5c, skipping...
      3. aefc78e - 169f729, skipping...
        • de64204, skipping...
        • 789491b, skipping...
        • 022b97a, skipping...
        • 27381ec, skipping...
        • 71dbe3b, skipping...
        • f338a5c, skipping...
      4. 27381ec - 169f729, skipping...
        • de64204, skipping...
        • 789491b, skipping...
        • 022b97a, skipping...
        • aefc78e, skipping...
        • 71dbe3b, skipping...
        • f338a5c, skipping...
      5. 71dbe3b - 169f729, skipping...
        • de64204, skipping...
        • 789491b, skipping...
        • 022b97a, skipping...
        • aefc78e, skipping...
        • 27381ec, skipping...
        • f338a5c, skipping...
      6. f338a5c - 169f729, skipping...
        • de64204, skipping...
        • 789491b, skipping...
        • 022b97a, skipping...
        • aefc78e, skipping...
        • 27381ec, skipping...
        • 71dbe3b, skipping... page_1|123456 page_2|223456 page_3|323456
leoheck commented 3 years ago

I was able to get a mac to test. There are issues with other programs too. For instance, find and sed and read, I am fixing this but it is not ready yet. I will let you know when you can do a second try

leoheck commented 3 years ago

I addressed the board name, it should be working now. I also fixed most of the issues with on OSX Please, install these too

brew install gsed
brew install findutils

Also, let me check if you have my last version of plotgitsch. Update it first and after, show me the output of this cat $(which git-imgdiff), please.

I am starting to add support for multiple schematic pages since my projects make use of this. This is not working yet but I am already displaying pages on the html on the schematic view.

Now, there is a flag to remove kidiff folder before executing the kdiff again (-r) and also one to dry run it without executing kidiff/plotgitsh/open (-d)

kdiff board.kicad_pcb -d -r

Also, the script is by default using a small list of changes where only commits that changes *.kicad_pcb and *.sch are being tracked to reduce the launch time. To enable it to the whole commit history, use the "all" flag (-a)

leoheck commented 3 years ago

Safari complains but schematic diffing does actually work on Firefox! Well done - great work!! Forget to comment on this. Safari should not complain. I am using the same comparing method we have on Kicad. This is why I saw a good opportunity to make this living mockup. Maybe it is complaining because something was not right on that version. Let's see after trying the last changes.

Thank you for the compliment. I appreciate :)

I think it is time to try to bring these textual diffs to the tool. I don't have much of an idea on how to do it. Do you have more examples from Altium to share? Those items are just diff from the SCM or are they something different?

leoheck commented 3 years ago

I was using my own project. But it is now pretty interesting to clone random projects on GitHub to check this thing. This one has a different board name as in yours.

https://github.com/tmk/HHKB_controller (here as an example of Kicad project)

image

leoheck commented 3 years ago

It is missing a legend with both colors saying

[blue] commit xyz345
[red] commit xyz123

Not fully functional but it is done. I want to update the hash of the commit next to the color.

Gasman2014 commented 3 years ago

See you have now fixed the legend too; something I had meant to do too.

It is really cool to be able to scroll through both sequential and arbitrary commits and all the layers! Looking really good on Firefox but images still don’t seem to load in Safari. I’ll take a look tomorrow.

Yes, I have a project with multiple schematic pages too and noticed that this wasn’t quite working but it is now managing to pick up the separate pages in the ‘Pages’ section.

I think it is time to try to bring these textual diffs to the tool. I don't have much of an idea on how to do it. Do you have more examples from Altium to share? Those items are just diff from the SCM or are they something different?

I don’t have any further info on the Altium diff - I am not sure about the best way to go about this. The diff text in the mockup does give info about individual components. The text diff that I wrote is not very sophisticated (and I think I have missed a couple of attributes - like ‘hidden’). My diff is a simple text diff between the two *.kicad_pcb files run through an elementary parser. What would make more sense would be to derive changes on a component level basis. So, for the schematic, between the symbols, text etc and their location and for the layout, between footprints, tracks, zones, edges, text etc. I recently started to have another look at this and did think that a more meaningful diff could be generated if the layouts were optimised for diffing i.e. generate a more useful format directly from KiCad.

On 24 Mar 2021, at 19:25, Leandro Heck @.***> wrote:

It is missing a legend with both colors saying

[blue] commit xyz345 [red] commit xyz123 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Gasman2014/KiCad-Diff/issues/36#issuecomment-806092803, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADFOMHLLQMQJJUA2NUGGTTFI4D3ANCNFSM4XYNIEMA.