Re: Harmful LESS flags
On 4/23/2014 5:11 PM, Jonathan Nieder wrote: That sounds like a nice feature request for 'less': a marker on the right margin when --chop-long-lines is in use and a line has been chopped. I don't see it at http://www.greenwoodsoftware.com/less/bugs.html#enhance so maybe no one else has thought of it yet. Mark, what do you think? Hi Jonathan, This seems reasonable. I actually thought that something like this was already implemented, and displayed in the status column when -J is in effect. But I must have dreamed that or something. I'll add this as an enhancement request. --Mark -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Junio C Hamano gits...@pobox.com writes: I am not opposed to changing the default in the longer term, as long as we have a solid transition plan to ensure that it won't disrupt and/or upset existing users too much. I am personally in favor of changing the default to drop the S. Silently hiding stuff from the user's eyes is really bad. With good coding standard and reasonable terminal size, it actually doesn't matter. And on projects actually containing very long lines (e.g. some people write LaTeX code with whole paragraphs for each lines), showing only the beginning of the line (i.e. the first line of the paragraph in my LaTeX example) isn't very useful. I do not think we particularly need a transition plan here: it's purely a user-interface thing, not something that may break any script or other tool. Changing the default and documenting the way to return to the old default in release notes and in the manual would be sufficient IMHO. GUI usually don't warn when the shape of a button is going to change in the next version ... -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Hi, Matthieu Moy wrote: I am personally in favor of changing the default to drop the S. Silently hiding stuff from the user's eyes is really bad. With good coding standard and reasonable terminal size, it actually doesn't matter. Just for clarity: no, when we are talking about well formatted code, -S is actually a way better interface. That's because indentation matters and makes it easy to take in code structure at a glance, long lines that get cut off by the margin stick out like a sore thumb already, and lines wrapped at an arbitrary character are even more distracting to the point of being useless. In practice I believe the Silently hiding stuff concern is much harder to solve. In the case of malicious code that opened this thread, I think a marker on the right margin would reveal the whitespace more clearly than wrapping that the reader may or may not notice. Luckily, it is very easy to switch between the two views on the fly --- in an already-open less window, you just type '-' + 'S'. In the spirit of not overriding tool defaults when there is not a strong reason to do so, I agree that if someone writes a patch to drop the 'S' I would probably like it. [...] I do not think we particularly need a transition plan here: it's purely a user-interface thing, not something that may break any script or other tool. Agreed a note in release notes and making sure the documentation reflects the new default should be enough. Thanks and hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Jonathan Nieder jrnie...@gmail.com writes: Matthieu Moy wrote: I am personally in favor of changing the default to drop the S. Silently hiding stuff from the user's eyes is really bad. With good coding standard and reasonable terminal size, it actually doesn't matter. Just for clarity: no, when we are talking about well formatted code, -S is actually a way better interface. When we are talking about well-formatted code, -S does not matter either which way. That's because indentation matters and makes it easy to take in code structure at a glance, long lines that get cut off by the margin stick out like a sore thumb already, and lines wrapped at an arbitrary character are even more distracting to the point of being useless. Lines which are cut off are not to the point of being useless, they _are_ useless. I am not arguing that wrapped lines are pretty. And I also consider the malicious or hiding angle at best a marginal concern. Overriding less' defaults should only be done for unequivocal benefits, and in this case I consider the result actually more of a detriment than anything else. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
David Kastrup wrote: Jonathan Nieder jrnie...@gmail.com writes: Just for clarity: no, when we are talking about well formatted code, -S is actually a way better interface. When we are talking about well-formatted code, -S does not matter either which way. Sorry for the lack of clarity. I believe well-formatted code can contain long lines. For example, sometimes a message + the printf to print it and indentation move past the right margin. If I wasn't talking about long lines, why would I have replied in the first place? [...] Overriding less' defaults should only be done for unequivocal benefits, We agree here. So, does someone who actually wants this change want to propose a patch? :) Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
David Kastrup d...@gnu.org writes: d...@mailtor.net writes: It would be nice if we could change the flags to either a) avoid cutting off b) indicate something has been cut off (- I prefer this) I assume there are more people with a similar workflow who're still unaware of this feature. I would joke about how 3 letter agencies introduced this flag to backdoor open source projects, but, well.. Most terminals are wider than three letters. I am having a hard time to decide if you genuinely misread what you are responding to, or if you are joking. If the latter, I find the joke mildly funny in a twisted way ;-) But the tangent aside... Still, it is a total nuisance. I am constantly doing -S RET on my git output. This should be left alone as an entirely personal preference quite unrelated to Git. There is no point in having Git configure a default different from what is used elsewhere. I almost agree with the general principle of the last sentence, but with a bit of reservation. The default value for LESS (i.e. when the user does not have any) we pass is FRSX, and the Porcelain output these days is colored by default. If we don't set a default at all, the end-user experience for a newbie will be bad, especially without R. Among the other three, F and X are to avoid a short output (e.g git show on a one-liner with a short explanation) from asking for confirmation to leave the pager and from clearing the screen upon leaving the pager, and are generally accepted as good things (or at least, we haven't seen much issue raised after we started passing the default LESS for those who do not have their own in their environment). Use of S is very subjective. While I personally do appreciate that we have it by default, I can perfectly well understand why some people do not want to see it in the default. The best we can do is to arrange so that people from one of the camps have their favorite out of the box and those from the other camp need to tell Git that they want to (or do not want to) fold long lines. Traditionally, because the tool grew in a context of being used in a project whose participants are at least not malicious, always having to be on the lookout for fear of middle-of-line tabs hiding bad contents near the right edges of lines has never been an issue. If somebody brought up a potential issue of such mode of attack back then, Linus may have chosen the default differently. I may have myself chosen not to have S, if I were the maintainer when the LESS default was originally introduced, and had I been made aware of this issue. I am not opposed to changing the default in the longer term, as long as we have a solid transition plan to ensure that it won't disrupt and/or upset existing users too much. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Junio C Hamano gits...@pobox.com writes: Traditionally, because the tool grew in a context of being used in a project whose participants are at least not malicious, always having to be on the lookout for fear of middle-of-line tabs hiding bad contents near the right edges of lines has never been an issue. My beef is not with hiding bad contents but with hiding contents. It makes the output useless for seeing what is actually happening as soon as the option starts having an effect. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: Traditionally, because the tool grew in a context of being used in a project whose participants are at least not malicious, always having to be on the lookout for fear of middle-of-line tabs hiding bad contents near the right edges of lines has never been an issue. My beef is not with hiding bad contents but with hiding contents. It makes the output useless for seeing what is actually happening as soon as the option starts having an effect. My suspicion is that one of the reasons why S was chosen to be in the default was to mildly discourage people from busting the usual line-length limit, but I am not Linus ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
On Thu, Apr 24, 2014 at 12:29:21PM -0700, Junio C Hamano wrote:
David Kastrup d...@gnu.org writes:
Junio C Hamano gits...@pobox.com writes:
Traditionally, because the tool grew in a context of being used in a
project whose participants are at least not malicious, always having
to be on the lookout for fear of middle-of-line tabs hiding bad
contents near the right edges of lines has never been an issue.
My beef is not with hiding bad contents but with hiding contents.
It makes the output useless for seeing what is actually happening as
soon as the option starts having an effect.
My suspicion is that one of the reasons why S was chosen to be in
the default was to mildly discourage people from busting the usual
line-length limit, but I am not Linus ;-)
I would think it's the opposite. Long lines look _horrible_ without
-S, as they get wrapped at awkward points. Using -S means that long
lines don't bug you, unless you really want to scroll over and see the
content.
I really think the right solution here is to teach less to make it more
obvious that there is something worth scrolling over to. Here's a very
rough patch for less, if you want to see what I'm thinking of.
diff --git a/input.c b/input.c
index b211323..01aa411 100755
--- a/input.c
+++ b/input.c
@@ -178,6 +178,7 @@ get_forw_line:
*/
if (chopline || hshift 0)
{
+ set_chopped_marker(ch_tell()-1);
do
{
if (ABORT_SIGS())
diff --git a/line.c b/line.c
index 1eb3914..b3358a0 100755
--- a/line.c
+++ b/line.c
@@ -1080,6 +1080,20 @@ set_status_col(c)
attr[0] = AT_NORMAL|AT_HILITE;
}
+ public void
+set_chopped_marker(pos)
+ POSITION pos;
+{
+ /*
+* Roll back output by one character; probably
+* we need to actually walk curr back further
+* for multibyte characters?
+*/
+ column--;
+ curr--;
+ store_char('', AT_NORMAL|AT_HILITE, NULL, pos);
+}
+
/*
* Get a character from the current line.
* Return the character as the function return value,
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Jeff King p...@peff.net writes: I would think it's the opposite. Long lines look _horrible_ without -S, as they get wrapped at awkward points. Using -S means that long lines don't bug you, unless you really want to scroll over and see the content. I really think the right solution here is to teach less to make it more obvious that there is something worth scrolling over to. Here's a very rough patch for less, if you want to see what I'm thinking of. Yes, I think that was suggested as an issue worth bringing up with less maintainers earlier in the thread already (and that was why I didn't repeat it). If we were in the business of updating less to suit many users' needs (the needs of our users included), we may even want to advocate turning R on by default. And I do agree that the chopped marker would be a very sensible thing to show in the -S output; I would have chosen $ myself for that to match an existing practice in (setq truncate-lines t) in Emacs, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
On Thu, Apr 24, 2014 at 02:47:01PM -0700, Junio C Hamano wrote: And I do agree that the chopped marker would be a very sensible thing to show in the -S output; I would have chosen $ myself for that to match an existing practice in (setq truncate-lines t) in Emacs, though. Hmm. I do not use Emacs, but I explicitly avoided $ because of its end-of-line connotations. E.g., in cat -A it means the opposite: this is the real \n end-of-line. But if there's existing precedent for $, that would be fine with me. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Jeff King p...@peff.net writes: On Thu, Apr 24, 2014 at 12:29:21PM -0700, Junio C Hamano wrote: David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: Traditionally, because the tool grew in a context of being used in a project whose participants are at least not malicious, always having to be on the lookout for fear of middle-of-line tabs hiding bad contents near the right edges of lines has never been an issue. My beef is not with hiding bad contents but with hiding contents. It makes the output useless for seeing what is actually happening as soon as the option starts having an effect. My suspicion is that one of the reasons why S was chosen to be in the default was to mildly discourage people from busting the usual line-length limit, but I am not Linus ;-) I would think it's the opposite. Long lines look _horrible_ without -S, as they get wrapped at awkward points. Using -S means that long lines don't bug you, unless you really want to scroll over and see the content. I prefer horrible over useless. I really think the right solution here is to teach less to make it more obvious that there is something worth scrolling over to. Here's a very rough patch for less, if you want to see what I'm thinking of. Still useless. I'm not actually interested in a more prominent I could be useful indicator. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
On Thu, Apr 24, 2014 at 11:48:30PM +0200, David Kastrup wrote: I really think the right solution here is to teach less to make it more obvious that there is something worth scrolling over to. Here's a very rough patch for less, if you want to see what I'm thinking of. Still useless. I'm not actually interested in a more prominent I could be useful indicator. So don't set -S, then. There are two questions here: 1. Can less do a better job of indicating what's in the input when -S is in effect? 2. What should get put into $LESS by default? I was specifically addressing (1). Your comment does not help at all there. It could have an impact on (2), but you didn't say anything besides I don't like it. That doesn't add anything to the conversation. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Jeff King p...@peff.net writes: On Thu, Apr 24, 2014 at 11:48:30PM +0200, David Kastrup wrote: I really think the right solution here is to teach less to make it more obvious that there is something worth scrolling over to. Here's a very rough patch for less, if you want to see what I'm thinking of. Still useless. I'm not actually interested in a more prominent I could be useful indicator. So don't set -S, then. I don't. Git does it unasked for. There are two questions here: 1. Can less do a better job of indicating what's in the input when -S is in effect? 2. What should get put into $LESS by default? I was specifically addressing (1). Your comment does not help at all there. It could have an impact on (2), but you didn't say anything besides I don't like it. That doesn't add anything to the conversation. No, I said it is useless, which is different from I don't like it. The information is not copypastable from a terminal window since it is cut off. It is also useless for review since one does not actually know what's in there. The only thing it has going for it is that it's prettier than the actually usable information. Which might sometimes be nice if one is not interested overly in the payload, like when using --graph. But then even a graph display wants to get copypasted into windows with a different size from the terminal window, like in URL:http://code.google.com/p/lilypond/issues/detail?id=3723#c7. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Hi, David Kastrup wrote: Jeff King p...@peff.net writes: There are two questions here: 1. Can less do a better job of indicating what's in the input when -S is in effect? 2. What should get put into $LESS by default? I was specifically addressing (1). Your comment does not help at all there. It could have an impact on (2), but you didn't say anything besides I don't like it. That doesn't add anything to the conversation. No, I said it is useless, which is different from I don't like it. The information is not copypastable from a terminal window since it is cut off. It is also useless for review since one does not actually know what's in there. The only thing it has going for it is that it's prettier than the actually usable information. I disagree with your characterization of what's useful here, but it really doesn't matter. Why are you still arguing? I think it would be fine to change git's default for LESS to FRX and document that change wherever the documentation currently mentions FRSX, if someone wants to write a patch for it. (Such a change would sit in pu or next until after 2.0.0 is released, of course.) In the meantime, when you're on machines using the current default, you have two choices: a) set the LESS envvar in your .profile explicitly b) hit the two keys '-', shift+S when git opens a pager The argument about safety is a red herring here, since it's always possible that a patch will wrap to make new lines with '+' or '-' or '@@' at the beginning that are equally confusing. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Harmful LESS flags
hello list,
as mentioned earlier on IRC, I'm a bit concerned about the default LESS flags
used by git.
The S option causes git to cut off everything to the right
Consider this diff, printed by `git diff`
#!/usr/bin/env python
-print('foo')
+print('bar')
Looks ok to merge and run.
But, after disabling the pager:
#!/usr/bin/env python
-print('foo')
+print('bar') [lots of tabs] ; import os; os.system('aptitude install
subversion')
Oh no!
My workflow is to clone a project, read the whole source and review all diffs
after fetching them. After that is done I merge origin into my local
branch and
run the code on my system.
I've panic'd a bit after I've noticed the chopping.
It would be nice if we could change the flags to either
a) avoid cutting off
b) indicate something has been cut off (- I prefer this)
I assume there are more people with a similar workflow who're still
unaware of
this feature.
I would joke about how 3 letter agencies introduced this flag to backdoor
open
source projects, but, well..
Sincerely yours,
a git user
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
(cc-ing Mark Nudelman, less maintainer)
Hi,
d...@mailtor.net wrote:
Consider this diff, printed by `git diff`
#!/usr/bin/env python
-print('foo')
+print('bar')
Looks ok to merge and run.
But, after disabling the pager:
Unfortunately there are other kinds of subtle bugs that can be hard to
see in a terminal, too.
[...]
It would be nice if we could change the flags to either
a) avoid cutting off
b) indicate something has been cut off (- I prefer this)
That sounds like a nice feature request for 'less': a marker on the
right margin when --chop-long-lines is in use and a line has been
chopped. I don't see it at
http://www.greenwoodsoftware.com/less/bugs.html#enhance so maybe no
one else has thought of it yet.
Mark, what do you think?
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
d...@mailtor.net writes: It would be nice if we could change the flags to either a) avoid cutting off b) indicate something has been cut off (- I prefer this) I assume there are more people with a similar workflow who're still unaware of this feature. I would joke about how 3 letter agencies introduced this flag to backdoor open source projects, but, well.. Most terminals are wider than three letters. Still, it is a total nuisance. I am constantly doing -S RET on my git output. This should be left alone as an entirely personal preference quite unrelated to Git. There is no point in having Git configure a default different from what is used elsewhere. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
