-
Notifications
You must be signed in to change notification settings - Fork 1k
Admin training: add some lines on how to apply a diff #6515
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
base: main
Are you sure you want to change the base?
Conversation
|
as a comment on how this was run previously, no one should ever see a real diff that's patchable. that's why we stripped the first character post syntax highlighting in JavaScript. then students could copy paste the changes without the + signs and without learning how to use the patch command, relying on their eyes and the syntax highlighting. We never used patch, it was intentionally avoided because it hurts the didactic experience of typing in the changes, or at minimum copying and pasting in the changed lines, and avoids the nightmare of what happens when they make their own customisations (as they should be doing) and patches don't apply cleanly. were I involved, I would recommend against this. As it is, I see + characters in diffs so, something clearly went wrong and wasn't caught earlier :( |
|
yeah :( , we broke/fixed it here #6503 |
|
So you want real diffs? Instead of our intentionally cropped ones? |
|
I guess I partially agree with this, in particular for less experienced people it might be an advantage that they have to navigate to the files and within the files repeatedly -- they will probably learn better what is where. If one is used to looking at diffs it should be fine. For me both is fine, but my fingers feel rather twisted after typing The possibility to switch between both displays would be perfect IMO. Sometimes patching is also faster (if one runs out of time). |
I agree this would be ideal, covers both use cases well. One could probably achieve this through some amount of javascript. |
|
@hexylena we didn't necessarily choose this, but before #6503 the copying was broken (gave you all the diff content, just without
@bernt-matthias use reverse search ;) |
by copying you mean the 'Copy' button that appears on hover? yeah, I don't think we ever envisioned someone using that to copy the diff contents, we envisioned them selecting the two added lines in a given diff with their mouse and pasting those in. I can see on the first diff the desire to hit 'copy' when it's creating a new file though 🤔 |
Maybe we should add an additional FAQ, but this way it will be shown everywhere where needed.