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

Why does GAP put two spaces after local and before then when printing functions? #473

Closed
fingolfin opened this issue Jan 16, 2016 · 7 comments · Fixed by #1498
Closed

Why does GAP put two spaces after local and before then when printing functions? #473

fingolfin opened this issue Jan 16, 2016 · 7 comments · Fixed by #1498
Labels
gapsagedays2016 Issues and PRs that arose at https://www.gapdays.de/gap-sage-days2016 kind: discussion discussions, questions, requests for comments, and so on
Milestone

Comments

@fingolfin
Copy link
Member

Consider this example:

gap> Display(function(x) local y; if x then return; fi; while y do od; end);
function ( x )
    local  y;
    if x  then
        return;
    fi;
    while y  do
        ;
    od;
    return;
end

Note the extra (second) space after local, and before do and then -- why are they there? Could we perhaps remove them?

@fingolfin fingolfin added the gapsagedays2016 Issues and PRs that arose at https://www.gapdays.de/gap-sage-days2016 label Jan 16, 2016
@fingolfin
Copy link
Member Author

The extra semicolon inside the empty while loop also seems a bit strange... Is it intentional? Could we get rid of it?

@stevelinton
Copy link
Contributor

Not sure about the spaces, but the ; reflects a No-op statement that is inserted into empty loops as they are parsed to avoid special-casing in the executor. Any really easy way to suppress printing would likely suppress explicit emty statements that actually were in the source.

@fingolfin
Copy link
Member Author

I think expressing empty statements that actually were in the source is something we would want to do anyway, as those empty statements shouldn't be there to begin with.

@frankluebeck
Copy link
Member

function ( x, y )
    local  z;
    if x = y  then
        z := 0;
    else
        z := x - y;
    fi;
    return z;
end

I wouldn't mind if the two spaces after local and before `then' would be reduced to one space.
But there are many other spaces which I find more (but still only slightly) disturbing:
Those around the '()'s in the first line, those in the too large indentation of 4 spaces per level, and those around arithmetic operators (well, the latter are fine if the expression fits easily on the line).

So, I would write:

function(x, y)
  local  z;
  if x = y  the      
    z := 0;
  else
    z := x-y;
  fi;
  return z;
end

I know other people who would prefer to put the whole if statement in one line if it is short enough.
But does this really matter, if the code is sufficiently readable?

@fingolfin
Copy link
Member Author

It does matter in so far as that new users and developers use the output of Display as a recommendation on how to format their code. And I am quite happy that we give them at least some suggestion, even if it is only in this indirect way.

Of course we could easily argue about how many spaces to use for indentation, and of course people have done so many, many times on countless occasions in the past decades -- I'd guess every minute, somewhere on this globe, somebody is arguing about that ;-).

Personally, I long ago learned to be flexible and adjust to the project at hand, and to adopt its formatting conventions, if there are any.

With GAP, there are none, not even recommendations, and that can be quite unfortunate, as soon as people mix different styles in a single function. I am less concerned if two separate functions use different styles, as long as they are readable -- so I guess in that regard, we agree :-).

Still, I was (and am) wondering about he double-spaces before do and then and after local. All other formatting variations I've seen in the GAP codebase resemble me of one or another formatting style I've seen for e.g. C. But usually the argument is about whether to put no space, or a single space, between things (and indention usually is debating tabs vs spaces, and 2 vs 4 for indention)

So, I am quite curious as to why those double spaces where put there. Is it just an accident, or on purpose?

And if nobody has a good reason, and nobody minds, I'd still like to remove them :-).

@olexandr-konovalov
Copy link
Member

+1 for removing extra spaces

@hungaborhorvath
Copy link
Contributor

I never would have thought to write two spaces anywhere else but as indentation. If I do, it is by accident. :-)

fingolfin added a commit to fingolfin/gap that referenced this issue Jul 12, 2017
In particular, don't print 'if x  then', but rather 'if x then'
(note the extra space in the former), and similar after "local",
and before "do".

Resolves gap-system#473
olexandr-konovalov pushed a commit to fingolfin/gap that referenced this issue Sep 13, 2017
In particular, don't print 'if x  then', but rather 'if x then'
(note the extra space in the former), and similar after "local",
and before "do".

Resolves gap-system#473
olexandr-konovalov pushed a commit to fingolfin/gap that referenced this issue Sep 14, 2017
In particular, don't print 'if x  then', but rather 'if x then'
(note the extra space in the former), and similar after "local",
and before "do". This resolves gap-system#473

Also, use consistent padding while printing IsBound statements,
update test files and add new test for printing IsBound calls
olexandr-konovalov pushed a commit to fingolfin/gap that referenced this issue Sep 14, 2017
In particular, don't print 'if x  then', but rather 'if x then'
(note the extra space in the former), and similar after "local",
and before "do". This resolves gap-system#473

Also, use consistent padding while printing IsBound statements,
update test files and add new test for printing IsBound calls,
as implemented by @alex-konovalov
ChrisJefferson pushed a commit that referenced this issue Sep 15, 2017
In particular, don't print 'if x  then', but rather 'if x then'
(note the extra space in the former), and similar after "local",
and before "do". This resolves #473

Also, use consistent padding while printing IsBound statements,
update test files and add new test for printing IsBound calls,
as implemented by @alex-konovalov
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Sep 15, 2017
@fingolfin fingolfin added kind: discussion discussions, questions, requests for comments, and so on and removed kind: question labels Mar 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gapsagedays2016 Issues and PRs that arose at https://www.gapdays.de/gap-sage-days2016 kind: discussion discussions, questions, requests for comments, and so on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants